Re: mkisofs hangs with -graft-points + -M

[Patrick Ohly <Patrick.Ohly@gmx.de>]

On Wed, 2004-01-07 at 12:58, Joerg Schilling wrote:
> >From: Patrick Ohly <Patrick.Ohly@gmx.de>
> 
> >Here's how this can be reproduced with cdrtools 2.01a23:
[...]
> >At this point mkisofs hangs and must be aborted.
> 
> There was a bug in mkisofs that caused the endless loop.
> It has been fixed in cdrtrools-2.01a24

Thanks. It seems I forgot to attach my patch for fixing
the problem to my original email, but it was trivial anyway.

I got another report from the same user. He pointed out
that my patch + backup script fail to recognize directories
with name length >= 147 as unmodified and writes their
content again.

This is because mkisofs' parse_rr() functions only copies
the first part of a directory entry name (after NM), which
may not be the full name: long names are split and the
remainder of the name is found in a CE entry.

This also leads to broken multisession CDs with the
unpatched mkisofs because it may fail to detect that an
existing directory entry in the old image is overwritten
with another one. Here's a sequence of commands that
produces such a broken CD with cdrtools 2.01a26pre:

mkdir -p
test/123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
touch
test/123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890/file1
mkisofs -R test | cdrecord dev=1,0,0 -multi -data -
rm
test/123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890/file1
touch
test/123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890/file2
mkisofs -M /dev/cdwriter -C 0,11702 -R test | cdrecord dev=1,0,0 -multi
-data -

Mounting the result shows:

root@gollum(~)# ll /cd
total 4
drwxr-sr-x    2 patrick  patrick      2048 Feb 15 18:52
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
drwxr-sr-x    2 patrick  patrick      2048 Feb 15 18:52
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
root@gollum(~)# ll
/cd/123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 
total 0
-rw-r--r--    1 patrick  patrick         0 Feb 15 18:52 file1

In other words, my CD contains two different directories with
the same name and I can only enter the first directory.

I think the attached patch fixes the problem. Instead of
aborting in parse_rr() after the first NM, it continues
to iterate and merges all NM records it finds.

Note that check_rr_dates() also called parse_rr().
I believe this is faulty and so I removed it in my
patch - see comments in patch for more information.

-- 
Bye, Patrick Ohly
--  
Patrick.Ohly@gmx.de
patrick@core.de (MakeCD related mails)
http://home.pages.de/~ohly/
http://makecd.core.de/ (MakeCD home page)

*** ../../cdrtools-2.01a26pre/mkisofs/multi.c	Wed Apr 30 00:18:10 2003
--- multi.c	Sun Feb 15 20:17:21 2004
***************
*** 461,471 ****
  		if (strncmp((char *) pnt, "NM", 2) == 0) {
  			strncpy(name_buf, (char *) pnt + 5, pnt[2] - 5);
  			name_buf[pnt[2] - 5] = 0;
! 			dpnt->name = strdup(name_buf);
! 			dpnt->got_rr_name = 1;
! 			return (0);
! 		}
! 		if (strncmp((char *) pnt, "CE", 2) == 0) {
  			cont_extent = isonum_733(pnt + 4);
  			cont_offset = isonum_733(pnt + 12);
  			cont_size = isonum_733(pnt + 20);
--- 461,478 ----
  		if (strncmp((char *) pnt, "NM", 2) == 0) {
  			strncpy(name_buf, (char *) pnt + 5, pnt[2] - 5);
  			name_buf[pnt[2] - 5] = 0;
! 			if (dpnt->name) {
! 				size_t len = strlen(dpnt->name);
! 				
! 				/* append to name from previous NM records */
! 				dpnt->name = realloc( dpnt->name, len + strlen(name_buf) + 1 );
! 				strcpy(dpnt->name + len, name_buf);
! 			} else {
! 				dpnt->name = strdup(name_buf);
! 				dpnt->got_rr_name = 1;
! 			}
! 			/* continue searching for more NM records */
! 		} else if (strncmp((char *) pnt, "CE", 2) == 0) {
  			cont_extent = isonum_733(pnt + 4);
  			cont_offset = isonum_733(pnt + 12);
  			cont_size = isonum_733(pnt + 20);
***************
*** 583,596 ****
  
  		len -= pnt[2];
  		pnt += pnt[2];
- 		if (len <= 3 && cont_extent) {
- 			unsigned char   sector[SECTOR_SIZE];
  
! 			readsecs(cont_extent, sector, 1);
! 			if (parse_rr(&sector[cont_offset],
! 							cont_size, dpnt) == -1)
! 				return (-1);
! 		};
  	};
  
  	/*
--- 590,614 ----
  
  		len -= pnt[2];
  		pnt += pnt[2];
  
! 		/*
! 		 * parse_rr() in this piece of code looks like
! 		 * a cut-and-paste error: what we would have to
! 		 * do is load the sector and search for PX and TF.
! 		 * parse_rr() won't do that, it overwrites the
! 		 * dpnt->name instead...
! 		 *
! 		 * Instead of doing the wrong thing, let's better
! 		 * not do anything - it worked in the past.
! 		 */
! /* 		if (len <= 3 && cont_extent) { */
! /* 			unsigned char   sector[SECTOR_SIZE]; */
! 
! /* 			readsecs(cont_extent, sector, 1); */
! /* 			if (parse_rr(&sector[cont_offset], */
! /* 							cont_size, dpnt) == -1) */
! /* 				return (-1); */
! /* 		}; */
  	};
  
  	/*