Re: [Libcdio-devel] [PATCH v3 0/2] Add Rock Ridge CE record processing

[Thomas Schmitt]

Hi,

i think that the chnages are ripe for being accepted and thus pushed
branch "pete_batard_ce_v3".
Rocky, please merge.

-------------------------------------------------------------------------
Tests made:

"git am" applied the v3 patches without complaints.
Both known problems of v2 are gone:

- The gcc complaint about mixed code and declarations does not appear
  any more.

- The run of iso-info with the ISO which contains the CE entries with
  offset larger than 255 does not SIGSEGV any more:

    iso-info --no-joliet \
             -i firmware-bookworm-DI-alpha1-amd64-netinst.iso \
             -l

  It produces 3892 output lines instead of less than 600 with v2.

  (I guess any Debian installation ISO from last year for amd64 with
   non-free firmware can serve as test object.
   But a recent change in debian-cd will avoid the softlinks which
   fill up a CE block by their target strings.)

The anticipated problem of patch v1 is indeed avoided:

- I verified by an intentionally manipulated ISO that the patch correctly
  handles the case that a CE entry is not the last entry in its system use
  area:

    iso-info --no-joliet -i iso9660_early_ce.iso

  yields
    2 /RockRidgeName:x
  The code in branch "master" yields the same name by just ignoring the
  CE entry. But the v1 patch would have yielded something derived from
  the dull name "ROCKRIDG.;1", because it would have hopped away from
  the NM entry which carries "RockRidgeName:x".

  The file iso9660_early_ce.iso was made for GRUB. That patch is still
  pending. Instructions how to craft this ISO are in
    https://lists.gnu.org/archive/html/grub-devel/2023-03/msg00032.html
  The file iso9660_early_ce.iso.gz is included as "IT binary patch" in
    https://lists.gnu.org/archive/html/grub-devel/2023-03/msg00034.html

-------------------------------------------------------------------------
Code review:

When reviewing the patch code i skipped over the white space changes in
patch 2, trusting that the compilers would catch any typos which were
inserted by random mistake. (I.e. finger errors, not brain errors.)

The effective code changes look good to me.

-----------------------------------------------------------------------

My cheat sheet says that i shall prepare a branch, push it, and ask
for merging. So i did:

  $ git push --set-upstream origin pete_batard_ce_v3
  Enter passphrase for key '[censored]':
  Counting objects: 13, done.
  Delta compression using up to 8 threads.
  Compressing objects: 100% (13/13), done.
  Writing objects: 100% (13/13), 2.38 KiB | 0 bytes/s, done.
  Total 13 (delta 11), reused 0 (delta 0)
  To scdbackup@git.sv.gnu.org:/srv/git/libcdio.git
   * [new branch]      pete_batard_ce_v3 -> pete_batard_ce_v3
  Branch pete_batard_ce_v3 set up to track remote branch pete_batard_ce_v3 from 
origin.


Have a nice day :)

Thomas