Hi,
Rocky Bernstein wrote:
> Overall, I'd like to get those patches without any controversy into the
> code soon. I am not 100% certain which ones have disputes in them. Is it
> just Patch 2?
Summary of everything that comes to my mind with v1. I read over v2 and
find not much changes in regard to my observations.
Pick what you deem important:
--------------------------------------------------------------------------
[PATCH 1/4] Add case insensitive _cdio_stricmp and _cdio_strnicmp function
calls:
- I have no objections. (But did not test explicitely beyond "git am"
and "make")
--------------------------------------------------------------------------
[PATCH 2/4] Add El Torito virtual boot image support:
- git complaint v1:
$ git am <patch_2
/.../.git/rebase-apply/patch:95: trailing whitespace.
/* Maximum number of El-Torito boot images we keep an index for */
warning: 1 line adds whitespace errors.
- About all legacy PC-BIOS boot images of Linux installation ISOs will
be shown with the recorded load size of 2048 bytes, although the
file size of these images (x86 programs) is in the range of several
dozen KiB.
There are few Linux installation ISOs without legacy boot image.
The real size of ISOLINUX and GRUB2 boot image is in the boot info
table record near the beginning of the image. (See man mkisofs or
doc/boot_sectors.txt in libisofs sources.)
- Some few EFI boot images of Linux installation ISOs will be shown with
the recorded sizes of 0 or 512 bytes, which according to UEFI means
"very large, assume it to reach up to the end of the device".
The real size is the size of the FAT filesystem in such an image.
v2:
This risk is avoided in v2 at the cost of the smaller risk of
encountering a legacy PC-BIOS image of size 1. (That was the size of
the image in the test ISO for v1.)
- Some few EFI boot images will be shown with size 32 MiB - 512 bytes,
although they are actually larger. (That's a flaw of the ISO, though.)
- The intended audience firmware (Platform Id in El Torito: legacy BIOS
or EFI) cannot be surely deduced from the presentation of the image
files in the /boot pseudo-directory. One would have to guess from
size and content.
On the other hand the boot image file name tells the emulation property
"noemul", which will always be the same with the images which get listed
by the patch. (Other images might emulate a floppy or a hard disk.)
- New observation:
I wonder about the uppercase/lowercase differences between the test ISO
from patch 4 and a Debian netinst ISO:
$ ./src/iso-info --no-header --quiet -l test/data/eltorito.iso
...
/[boot]/:
- [LSN 26] 512 Jan 23 2024 13:25:09 0-boot-noemul.img
...
$ ./src/iso-info --no-header --quiet -l debian-12.2.0-amd64-netinst.iso
...
/[BOOT]/:
- [LSN 5863] 2048 Oct 07 2023 10:32:09 0-Boot-NoEmul.img
- [LSN 1119] 9715712 Oct 07 2023 10:32:09 1-Boot-NoEmul.img
...
The test ISO has no Rock Ridge or Joliet. Is that enough reason to
flatten the pseudo-file names ?
If so: Does this meet Pete's usage intentions ?
--------------------------------------------------------------------------
[PATCH 3/4] Add --no-el-torito option to iso-info
- In my own projects i would rather disable the new feature by default
and offer an option --el-torito.
--------------------------------------------------------------------------
[PATCH 4/4] Add El Torito tests
- git complaint v1:
$ git am <patch_4
Applying: Add El Torito tests
/.../.git/rebase-apply/patch:69: trailing whitespace.
# Tests El Torito (via iso-info and iso-read).
/../.git/rebase-apply/patch:158: new blank line at EOF.
+
/.../.git/rebase-apply/patch:172: new blank line at EOF.
+
warning: 3 lines add whitespace errors.
- New objection:
The proposed ISO image is very unrealistic by the load size 1 of its
only boot image. Especially it does not test the use case which caused
Pete to develop the patch set and which has chances to show realistic
sizes: An EFI boot image, which contains a FAT filesystem.
(One could make an ISO with a real isolinux.bin from Debian netinst ISO
as legacy boot image of 38912 bytes and a minimal FAT filesystem as
EFI boot image.)
--------------------------------------------------------------------------
> if Thomas you have the
> inclination and time to make more perfect, that can be done as a follow on.
Some of my objections are about design decisions. Pete strives for
compatibility with 7z rather than for exposing all El Torito aspects.
Further the patch knowingly risks wrong results.
So we would first have to find some more common ground before it makes
sense that i do more than proof-reading.
Have a nice day :)
Thomas
