Re: [Libcdio-devel] [RFC] New API iso9660_statv2_t as API/ABI compatible way to read files >= 4 GiB

[Pete Batard]

On 2018.07.09 17:17, Thomas Schmitt wrote:
> The reader from ISO directory records, _iso9660_dir_to_statbuf(), knows
> the start and size of the previous extent. So it could check whether the
> current extent fits seamlessly.
> E.g. after:
>
>      p_stat->extent_lsn[p_stat->num_extents] = from_733 (p_iso9660_dir->extent);
>      p_stat->extent_size[p_stat->num_extents] = from_733 (p_iso9660_dir->size);
>      p_stat->total_size += p_stat->extent_size[p_stat->num_extents];
>
> it could do:
>
>      if (p_stat->extent_lsn[p_stat->num_extents - 1]
>          + p_stat->extent_size[p_stat->num_extents - 1] / ISO_BLOCKSIZE)
>          != p_stat->extent_lsn[p_stat->num_extents]
>          ||
>          p_stat->extent_size[p_stat->num_extents - 1] % ISO_BLOCKSIZE) {
>
>        /* >>> Gap detected <<< */;
>
>      }

Ah, now I'm starting to see what your actual plan is.

My understanding was that you were planning to drop the extent_lsn[] and 
extent_size[] members altogether, keeping only the first extent lsn and 
the last extent size (for last extent truncation), along with a new 
total_size, and assert that everything would be set sequentially in 4 GB 
extents in between, dropping the need for ISO_MAX_MULTIEXTENT (since we 
would no longer have the arrays).

If you are planning to keep the arrays, then I guess your whole point is 
about avoiding libcdio users to add an inner extent loop if they prefer.

I personally don't believe it's that big a deal, but, if we are not 
going to drop the publicly exposed LSN arrays, I don't have much of an 
objection providing samples that go in this direction (while having at 
least one sample that does the inner loop, along with 
alignment/truncation handling, since we're going to have the extent 
arrays exposed anyway), and give the choice to downstream folks to pick 
the one they prefer for dealing with multiextent.

> If gaps are found, it could either bail out with error message, or it
> could mark the stat buffer by a new flag member. Another flag could
> indicate whether ISO_MAX_MULTIEXTENT was exceeded.

Bail out is what I would advocate for. I don't like the idea of giving 
the opportunity for people to have a big chunk of data missing or 
corrupted, and go on their merry way if they fail to check a flag. 
Missing/corrupted data _is_ a big deal and should result in an error.

Apart from that, and now that I think I understand what you're trying to 
accomplish, sure, adding an alternate path in our code to read a 
multiextent file in one sway sounds like a good idea, if it doesn't add 
too much complexity.

But considering that we still need to have some arrays of extents LSNs 
and sizes (either dynamic or fixed), it still doesn't do much to answer 
the question about using a _v2 and keeping the ABI, or just expending 
the existing struct and breaking the ABI (reminding everyone that none 
of these options would break the API). Unless I'm mistaken, what you are 
proposing can be accomplished on top of whichever option we pick.

Regards,

/Pete