Re: [Libcdio-devel] [PATCH 2/2] Add processing of Rock Ridge CE records

[Pete Batard]

Hi Thomas,

On 2023.02.28 21:47, Thomas Schmitt wrote:
> Hi,
>
> Pete Batard wrote:
> > Use the processed cont_extent to read the relevant continuation block
> > into a buffer and then use that block, along with cont_offset and
> > cont_size, to carry out further Rock Ridge data processing.
> > [...]
> > +       if (iso9660_iso_seek_read(p_image, buffer, cont_extent, 1) != 
> > ISO_BLOCKSIZE)
> > +         goto out;
> > +       chr = &buffer[cont_offset];
> > +       len = cont_size;
>
> Do i get it right that encountering a CE entry causes immediate
> continuation of interpretation at the depicted continuation area ?

Yes, that is what the proposal does, since what I could see from Debian 
ISO I played along with the name of the field appeared to hint that a CE 
would be the last entry, and I didn't really bother checking the SUSP specs.

> If so, then this does not obey the specs. SUSP 1.12 says:
>
>    The "CE" System Use Entry indicates a Continuation Area that shall be
>    processed after the current System Use field or Continuation Area is
>    processed.
>
> So the CE parameters cont_extent, cont_offset, cont_size need to be
> buffered until the while-loop has ended. Only then the continuation
> area should be loaded and interpreted.

That's a good point.

I'll see if I can spare some time to fix the code and send a v2 that 
delays CE processing, but I have to say that, without an immediate image 
that requires this in Rufus, it's possible that I may never get around 
to it especially as, and this was initial assumption, I'd expect most 
Rock Ridge mastering tools to place the CE at the end, since it's 
expected to be used when they're running out of space in the current 
record...

I'll see what I can do, but I can't promise anything at this stage.

> Linux and libisofs do this buffering when interpreting CE. GRUB does not.
>
> mkisofs, genisoimage, and libisofs write CE as last entry, so that GRUB
> did not experience problems yet. (I plan to craft a demonstration ISO
> and to submit a patch to GRUB.)
>
> --------------------------------------------------------------------
> Further:
>
> Consider to implement interpretation of the ST entry, which marks the
> end of the SUSP data within a directory record or continuation area.
> I.e. it should break the while-loop regardless of the remaining len.
>
> SUSP 1.12 says
>
>    If the remaining allocated space following the last recorded System
>    Use Entry in a System Use field or Continuation Area is less than four
>    bytes long, it cannot contain a System Use Entry and shall be ignored.
>    Otherwise an "ST" System Use Entry (see section 5.4) shall be the last
>    System Use Entry recorded in a System Use field or Continuation Area.
>
> So the current while-loop condition
>
>      while (len > 1){ /* There may be one byte for padding somewhere */
>
> could be widened to (len >= 4) ... if i am not off by one.

At least that one will be easier to address. :)

Thanks a lot for reviewing this proposal!

/Pete

> Have a nice day :)
>
> Thomas