Andy Polyakov <[EMAIL PROTECTED]> wrote: > Fix for multi-extent bug in multi.c has [at least] two bugs: > > 1. If directory entries constituting multi-extent file end up in > different sectors read_merging_directory ends up in endless loop. > Problematic loop is denoted in attached diff along with possible solution.
Coiuld you please explant your claim? The code you removed is needed and you did not provide a working replacement. > 2. Given that mkisofs reserves for file system layout generated by > another iso9660 formatter, alternative partitioning of file to multiple > extents (for example larger amount of smaller extents) for now would > result in effective data corruption. Point is that it's not enough to This is a claim that would need a prove... > copy extents' isorec.extent fields alone, one has to copy at least > isorec.size fields as well. As for "at least." *Formally* (i.e. knowing > nothing about alternative iso9660 formatting and assuming "worst") one > should copy even isorec.ext_attr_length, [some bits from] flags, > file_unit_size and interleave. Attached diff denotes problematic code by > simply copying all of above mentioned fields in single memcpy (along > with isorec.date field, which is checked to be same as in curr_entry, so > copying should not cause side effects). As your patch does not handle the file correctly anymore, we would need to discuss it. > Couple of comments not related to above file-system formatting bugs. > > There is inconsistency in directory_entry.mxpart declaration and its > usage. Declaration is "guarded" by #ifdef USE_LARGEFILES, while its > usage is not. Formally multi-extent files don't have to be "large," i.e. > "small" files are allowed to be multi-extent too (actually original > application for multi-extent was *CD* packet writing). Therefore it, > multi-extent support, should not be associated with USE_LARGEFILES and > mxpart declaration should not be depend on corresponding macro. The problem with multi extent files in ISO-9660 is that it allows you to any kind of nonsense. > In check_prev_session one can see free(ptr[i]); and in copy_mult_ext one > can see free(sex);. These are pointers to directory_entry structure, > which in turn can contain pointers to other dynamically allocated > objects, most notably rr_attributes and name. Freeing an object without > freeing other dynamically allocated objects it alone refers to used to > constitute memory leaks. > > As for copy_mult_extent. As mentioned above it copies pieces of > information from original extent records. As alternative to this field > copying would it be appropriate to consider simply using original extent > records, i.e. removing them from orig_contents array and linking them > into this_dir? A. > > P.S. As usual I make no claims on correctness of provided patch, it > serves primarily educational purposes. I am currently at OSCON and don't have the time for a closer look. Let us continue next week. Jörg -- EMail:[EMAIL PROTECTED] (home) Jörg Schilling D-13353 Berlin [EMAIL PROTECTED] (uni) [EMAIL PROTECTED] (work) Blog: http://schily.blogspot.com/ URL: http://cdrecord.berlios.de/old/private/ ftp://ftp.berlios.de/pub/schily -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]