Re: [Libcdio-devel] Hindsight is 2020 - Let's sort multiextents at last

[Thomas Schmitt]

Hi,

the changes in revise-examples-for-multi-extent look good to me.

But there might be some more occasions of ->size which need change.
I did:
  fgrep -r '>size' . | less
and tried to classify the found code snippets.

Please review.

Further we need to think about proper announcement of the change.
I quote some older text of me for mining at the end of this mail.

-----------------------------------------------------------------------------
Occasions where i think that ->total_size should be used:

-------------------------
./example/C++/isofile.cpp:
  if (ftruncate (fileno (p_outfd), p_statbuf->size))

p_statbuf comes from iso9660_ifs_stat_translate().
It is used in the same (main) function as
   CEILING(p_statbuf->total_size, ISO_BLOCKSIZE)

So i expect that the ftruncate() call should get ->total_size too.

--------------------------
./example/C++/OO/iso4.cpp:
  p_s->p_stat->lsn, p_s->p_stat->size, psz_path, filename);

Here i stand somewhat in the woods because p_s is obtained by some C++
specific iteration. But its ->p_stat seems not to be a stat(2) struct.
So i assume that ->total_size would be appropriate.

-------------------
./doc/libcdio.texi:
  for (i = 0; i < p_statbuf->size; i += ISO_BLOCKSIZE)

Not active code, but clearly in need of modernization.


-----------------------------------------------------------------------------
fgrep matches which to my opinion need no change:

./lib/driver/image/nrg.c:
  env->size = MAX (env->size, (start_lsn + sec_count));
  ... and more of these ...
env is _img_private_t.

./lib/iso9660/iso9660.c:
  uint32_t dsize = from_733(idr->size);
  ... and more of these ...
idr is iso9660_dir_t.

./lib/iso9660/iso9660_fs.c:
  p_stat->total_size += from_733(p_iso9660_dir->size);
  p_stat->size = from_733(p_iso9660_dir->size);
  p_stat->secsize = CDIO_EXTENT_BLOCKS(p_stat->size);
These are part of the augmentation for multi-extent and thus appropriate.

=========================================================================

The old applications would still work for single-extent files.
But libcdio should announce the new ->total_size quite loudly.

Maybe one could strip down my description from summer 2018:
-------------------------------------------------------------------------

pragmatic-multiextent [...]
can create iso9660_stat_t objects only from files of which the
extents form a sequentially readable byte string.

If this expectation is not fulfilled, then pragmatic-multiextent issues
a warning message and does not create an iso9660_stat_t object for the
affected file. I.e. the file will not appear in readdir()-ish results and
individual inquiries will yield NULL rather than an iso9660_stat_t object.

mkisofs and libisofs currently only produce the expected simple extent
layout. Up to now, no other kind of multi-extent files have been found
in the wild. (I had to fake a test ISO with unaligned extent size.
An ISO where extents hop forth and back has still to be counterfeit.)

[... omit statements about ABI breakage and regression probability ...]

[Instead, one should mention:
 The old libcdio applications will still work for single-extent files.
 But if they shall handle large multi-extent files they need to be
 adapted:
]

Applications only have to switch from using
  iso9660_stat_t.size
to using new member
  iso9660_stat_t.total_size
and to make sure that their loop controlling variables can cope with
integer numbers up to 43 bit (32 bit block address, 11 bit byte address in
block).

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

Have a nice day :)

Thomas