On Mon, May 25, 2020 at 4:23 AM Thomas Schmitt <scdbac...@gmx.net> wrote:
> 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. > Thanks for looking over. Indeed as often the case with my changes, there is much more to be desired. > > 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. > Ok. I don't know what the best way to do this is though. Suggestions? > > > ----------------------------------------------------------------------------- > Occasions where i think that ->total_size should be used: > > ------------------------- > ./example/C++/isofile.cpp: > if (ftruncate (fileno (p_outfd), p_statbuf->size)) > Should be fixed in commit d10f3a4d > > 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. > Right. Should be fixed in commit d10f3a4d > > -------------------------- > ./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. > Changed in d10f3a4d. Seems to work for now ;-) > ------------------- > ./doc/libcdio.texi: > for (i = 0; i < p_statbuf->size; i += ISO_BLOCKSIZE) > > Not active code, but clearly in need of modernization. > Have revised in d10f3a4d. Thanks for catching. > > > > ----------------------------------------------------------------------------- > 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. > Ok. Not sure where to go. The usual thing is to add this to NEWS.md > > 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). > > In commit 32785270 I added this: version 3.0.0 > ============= > This version adds multi-extent support for ISO-9660. Previously the blocks > making up an ISO-9660 file had to come strictly sequentially. > In this version, when libiso9600 detects that this is not the case, a > warning message is generated and a `iso9660_stat_t` object is not created. > the file will not appear in `readdir()`-ish results and individual > inquiries will yield `NULL` rather than an `iso9660_stat_t` object. > Programs written for previous versions of _libcdio_ will still work for > single-extent files. However in order to handle large multi-extent files, > applications > need to be adapted: > * switch from using `iso9660_stat_t.size` to the new attrtibute > `iso9660_stat_t.total_size` > * make sure that loop controlling variables can cope with integer numbers > up to 43 bit numbers. In C-like lanaguage this may mean switching from > `int` to `long int` > Notes: 43 bits is the sum of a 32-bit block address plus an 11-bit byte > address in a block); `mkisofs` and `libisofs` currently only produce the > files with sequential blocks. Feel free to revise or comment on any of the changes just made in that branch. It's okay just to directly commit in that branch. ------------------------------------------------------------------------- > > Have a nice day :) > > Thomas > > >
