[PATCH 0/9] isofs: unobfuscate rock.c

[PATCH 0/9] isofs: unobfuscate rock.c

[Pekka Enberg]

This patch removes macro obfuscation from fs/isofs/rock.c and cleans it up
a bit to make it more readable and maintainable. There are no functional
changes, only cleanups. I have only tested this lightly but it passes
mount and read on small Rock Ridge enabled ISO image.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>

[PATCH 1/9] isofs: indent rock.c

[Pekka Enberg]

This patch indents the file fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |  647 +++++++++++++++++++++++++++++++++++------------------------------
 1 files changed, 356 insertions(+), 291 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:26:26.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:27:05.000000000 +0300
@@ -26,8 +26,7 @@
  * returns a symbolic link name, and a fourth one returns the extent number
  * for the file. */
 
-#define SIG(A,B) ((A) | ((B) << 8)) /* isonum_721() */
-
+#define SIG(A,B) ((A) | ((B) << 8))	/* isonum_721() */
 
 /* This is a way of ensuring that we have something in the system
    use fields that is compatible with Rock Ridge */
@@ -60,7 +59,7 @@
      CHR+=ISOFS_SB(inode->i_sb)->s_rock_offset;                \
      if (LEN<0) LEN=0;                                          \
   }                                                             \
-}                                     
+}
 
 #define MAYBE_CONTINUE(LABEL,DEV) \
   {if (buffer) { kfree(buffer); buffer = NULL; } \
@@ -91,280 +90,348 @@
   }}
 
 /* return length of name field; 0: not found, -1: to be ignored */
-int get_rock_ridge_filename(struct iso_directory_record * de,
-			    char * retname, struct inode * inode)
+int get_rock_ridge_filename(struct iso_directory_record *de,
+			    char *retname, struct inode *inode)
 {
-  int len;
-  unsigned char * chr;
-  CONTINUE_DECLS;
-  int retnamlen = 0, truncate=0;
- 
-  if (!ISOFS_SB(inode->i_sb)->s_rock) return 0;
-  *retname = 0;
-
-  SETUP_ROCK_RIDGE(de, chr, len);
- repeat:
-  {
-    struct rock_ridge * rr;
-    int sig;
-    
-    while (len > 2){ /* There may be one byte for padding somewhere */
-      rr = (struct rock_ridge *) chr;
-      if (rr->len < 3) goto out; /* Something got screwed up here */
-      sig = isonum_721(chr);
-      chr += rr->len; 
-      len -= rr->len;
-      if (len < 0) goto out;	/* corrupted isofs */
-
-      switch(sig){
-      case SIG('R','R'):
-	if((rr->u.RR.flags[0] & RR_NM) == 0) goto out;
-	break;
-      case SIG('S','P'):
-	CHECK_SP(goto out);
-	break;
-      case SIG('C','E'):
-	CHECK_CE;
-	break;
-      case SIG('N','M'):
-	if (truncate) break;
-	if (rr->len < 5) break;
-        /*
-	 * If the flags are 2 or 4, this indicates '.' or '..'.
-	 * We don't want to do anything with this, because it
-	 * screws up the code that calls us.  We don't really
-	 * care anyways, since we can just use the non-RR
-	 * name.
-	 */
-	if (rr->u.NM.flags & 6) {
-	  break;
-	}
+	int len;
+	unsigned char *chr;
+	CONTINUE_DECLS;
+	int retnamlen = 0, truncate = 0;
 
-	if (rr->u.NM.flags & ~1) {
-	  printk("Unsupported NM flag settings (%d)\n",rr->u.NM.flags);
-	  break;
-	}
-	if((strlen(retname) + rr->len - 5) >= 254) {
-	  truncate = 1;
-	  break;
+	if (!ISOFS_SB(inode->i_sb)->s_rock)
+		return 0;
+	*retname = 0;
+
+	SETUP_ROCK_RIDGE(de, chr, len);
+      repeat:
+	{
+		struct rock_ridge *rr;
+		int sig;
+
+		while (len > 2) {	/* There may be one byte for padding somewhere */
+			rr = (struct rock_ridge *)chr;
+			if (rr->len < 3)
+				goto out;	/* Something got screwed up here */
+			sig = isonum_721(chr);
+			chr += rr->len;
+			len -= rr->len;
+			if (len < 0)
+				goto out;	/* corrupted isofs */
+
+			switch (sig) {
+			case SIG('R', 'R'):
+				if ((rr->u.RR.flags[0] & RR_NM) == 0)
+					goto out;
+				break;
+			case SIG('S', 'P'):
+				CHECK_SP(goto out);
+				break;
+			case SIG('C', 'E'):
+				CHECK_CE;
+				break;
+			case SIG('N', 'M'):
+				if (truncate)
+					break;
+				if (rr->len < 5)
+					break;
+				/*
+				 * If the flags are 2 or 4, this indicates '.' or '..'.
+				 * We don't want to do anything with this, because it
+				 * screws up the code that calls us.  We don't really
+				 * care anyways, since we can just use the non-RR
+				 * name.
+				 */
+				if (rr->u.NM.flags & 6) {
+					break;
+				}
+
+				if (rr->u.NM.flags & ~1) {
+					printk
+					    ("Unsupported NM flag settings (%d)\n",
+					     rr->u.NM.flags);
+					break;
+				}
+				if ((strlen(retname) + rr->len - 5) >= 254) {
+					truncate = 1;
+					break;
+				}
+				strncat(retname, rr->u.NM.name, rr->len - 5);
+				retnamlen += rr->len - 5;
+				break;
+			case SIG('R', 'E'):
+				if (buffer)
+					kfree(buffer);
+				return -1;
+			default:
+				break;
+			}
+		}
 	}
-	strncat(retname, rr->u.NM.name, rr->len - 5);
-	retnamlen += rr->len - 5;
-	break;
-      case SIG('R','E'):
-	if (buffer) kfree(buffer);
-	return -1;
-      default:
-	break;
-      }
-    }
-  }
-  MAYBE_CONTINUE(repeat,inode);
-  if (buffer) kfree(buffer);
-  return retnamlen; /* If 0, this file did not have a NM field */
- out:
-  if(buffer) kfree(buffer);
-  return 0;
+	MAYBE_CONTINUE(repeat, inode);
+	if (buffer)
+		kfree(buffer);
+	return retnamlen;	/* If 0, this file did not have a NM field */
+      out:
+	if (buffer)
+		kfree(buffer);
+	return 0;
 }
 
 static int
 parse_rock_ridge_inode_internal(struct iso_directory_record *de,
 				struct inode *inode, int regard_xa)
 {
-  int len;
-  unsigned char * chr;
-  int symlink_len = 0;
-  CONTINUE_DECLS;
-
-  if (!ISOFS_SB(inode->i_sb)->s_rock) return 0;
-
-  SETUP_ROCK_RIDGE(de, chr, len);
-  if (regard_xa)
-   {
-     chr+=14;
-     len-=14;
-     if (len<0) len=0;
-   }
-   
- repeat:
-  {
-    int cnt, sig;
-    struct inode * reloc;
-    struct rock_ridge * rr;
-    int rootflag;
-    
-    while (len > 2){ /* There may be one byte for padding somewhere */
-      rr = (struct rock_ridge *) chr;
-      if (rr->len < 3) goto out; /* Something got screwed up here */
-      sig = isonum_721(chr);
-      chr += rr->len; 
-      len -= rr->len;
-      if (len < 0) goto out;	/* corrupted isofs */
-      
-      switch(sig){
+	int len;
+	unsigned char *chr;
+	int symlink_len = 0;
+	CONTINUE_DECLS;
+
+	if (!ISOFS_SB(inode->i_sb)->s_rock)
+		return 0;
+
+	SETUP_ROCK_RIDGE(de, chr, len);
+	if (regard_xa) {
+		chr += 14;
+		len -= 14;
+		if (len < 0)
+			len = 0;
+	}
+
+      repeat:
+	{
+		int cnt, sig;
+		struct inode *reloc;
+		struct rock_ridge *rr;
+		int rootflag;
+
+		while (len > 2) {	/* There may be one byte for padding somewhere */
+			rr = (struct rock_ridge *)chr;
+			if (rr->len < 3)
+				goto out;	/* Something got screwed up here */
+			sig = isonum_721(chr);
+			chr += rr->len;
+			len -= rr->len;
+			if (len < 0)
+				goto out;	/* corrupted isofs */
+
+			switch (sig) {
 #ifndef CONFIG_ZISOFS		/* No flag for SF or ZF */
-      case SIG('R','R'):
-	if((rr->u.RR.flags[0] & 
- 	    (RR_PX | RR_TF | RR_SL | RR_CL)) == 0) goto out;
-	break;
+			case SIG('R', 'R'):
+				if ((rr->u.RR.flags[0] &
+				     (RR_PX | RR_TF | RR_SL | RR_CL)) == 0)
+					goto out;
+				break;
 #endif
-      case SIG('S','P'):
-	CHECK_SP(goto out);
-	break;
-      case SIG('C','E'):
-	CHECK_CE;
-	break;
-      case SIG('E','R'):
-	ISOFS_SB(inode->i_sb)->s_rock = 1;
-	printk(KERN_DEBUG "ISO 9660 Extensions: ");
-	{ int p;
-	  for(p=0;p<rr->u.ER.len_id;p++) printk("%c",rr->u.ER.data[p]);
-	}
-	  printk("\n");
-	break;
-      case SIG('P','X'):
-	inode->i_mode  = isonum_733(rr->u.PX.mode);
-	inode->i_nlink = isonum_733(rr->u.PX.n_links);
-	inode->i_uid   = isonum_733(rr->u.PX.uid);
-	inode->i_gid   = isonum_733(rr->u.PX.gid);
-	break;
-      case SIG('P','N'):
-	{ int high, low;
-	  high = isonum_733(rr->u.PN.dev_high);
-	  low = isonum_733(rr->u.PN.dev_low);
-	  /*
-	   * The Rock Ridge standard specifies that if sizeof(dev_t) <= 4,
-	   * then the high field is unused, and the device number is completely
-	   * stored in the low field.  Some writers may ignore this subtlety,
-	   * and as a result we test to see if the entire device number is
-	   * stored in the low field, and use that.
-	   */
-	  if((low & ~0xff) && high == 0) {
-	    inode->i_rdev = MKDEV(low >> 8, low & 0xff);
-	  } else {
-	    inode->i_rdev = MKDEV(high, low);
-	  }
-	}
-	break;
-      case SIG('T','F'):
-	/* Some RRIP writers incorrectly place ctime in the TF_CREATE field.
-	   Try to handle this correctly for either case. */
-	cnt = 0; /* Rock ridge never appears on a High Sierra disk */
-	if(rr->u.TF.flags & TF_CREATE) { 
-	  inode->i_ctime.tv_sec = iso_date(rr->u.TF.times[cnt++].time, 0);
-	  inode->i_ctime.tv_nsec = 0;
-	}
-	if(rr->u.TF.flags & TF_MODIFY) {
-	  inode->i_mtime.tv_sec = iso_date(rr->u.TF.times[cnt++].time, 0);
-	  inode->i_mtime.tv_nsec = 0;
-	}
-	if(rr->u.TF.flags & TF_ACCESS) {
-	  inode->i_atime.tv_sec = iso_date(rr->u.TF.times[cnt++].time, 0);
-	  inode->i_atime.tv_nsec = 0;
-	}
-	if(rr->u.TF.flags & TF_ATTRIBUTES) { 
-	  inode->i_ctime.tv_sec = iso_date(rr->u.TF.times[cnt++].time, 0);
-	  inode->i_ctime.tv_nsec = 0;
-	} 
-	break;
-      case SIG('S','L'):
-	{int slen;
-	 struct SL_component * slp;
-	 struct SL_component * oldslp;
-	 slen = rr->len - 5;
-	 slp = &rr->u.SL.link;
-	 inode->i_size = symlink_len;
-	 while (slen > 1){
-	   rootflag = 0;
-	   switch(slp->flags &~1){
-	   case 0:
-	     inode->i_size += slp->len;
-	     break;
-	   case 2:
-	     inode->i_size += 1;
-	     break;
-	   case 4:
-	     inode->i_size += 2;
-	     break;
-	   case 8:
-	     rootflag = 1;
-	     inode->i_size += 1;
-	     break;
-	   default:
-	     printk("Symlink component flag not implemented\n");
-	   }
-	   slen -= slp->len + 2;
-	   oldslp = slp;
-	   slp = (struct SL_component *) (((char *) slp) + slp->len + 2);
-
-	   if(slen < 2) {
-	     if(    ((rr->u.SL.flags & 1) != 0) 
-		    && ((oldslp->flags & 1) == 0) ) inode->i_size += 1;
-	     break;
-	   }
-
-	   /*
-	    * If this component record isn't continued, then append a '/'.
-	    */
-	   if (!rootflag && (oldslp->flags & 1) == 0)
-		   inode->i_size += 1;
-	 }
-	}
-	symlink_len = inode->i_size;
-	break;
-      case SIG('R','E'):
-	printk(KERN_WARNING "Attempt to read inode for relocated directory\n");
-	goto out;
-      case SIG('C','L'):
-	ISOFS_I(inode)->i_first_extent = isonum_733(rr->u.CL.location);
-	reloc = isofs_iget(inode->i_sb, ISOFS_I(inode)->i_first_extent, 0);
-	if (!reloc)
-		goto out;
-	inode->i_mode = reloc->i_mode;
-	inode->i_nlink = reloc->i_nlink;
-	inode->i_uid = reloc->i_uid;
-	inode->i_gid = reloc->i_gid;
-	inode->i_rdev = reloc->i_rdev;
-	inode->i_size = reloc->i_size;
-	inode->i_blocks = reloc->i_blocks;
-	inode->i_atime = reloc->i_atime;
-	inode->i_ctime = reloc->i_ctime;
-	inode->i_mtime = reloc->i_mtime;
-	iput(reloc);
-	break;
+			case SIG('S', 'P'):
+				CHECK_SP(goto out);
+				break;
+			case SIG('C', 'E'):
+				CHECK_CE;
+				break;
+			case SIG('E', 'R'):
+				ISOFS_SB(inode->i_sb)->s_rock = 1;
+				printk(KERN_DEBUG "ISO 9660 Extensions: ");
+				{
+					int p;
+					for (p = 0; p < rr->u.ER.len_id; p++)
+						printk("%c", rr->u.ER.data[p]);
+				}
+				printk("\n");
+				break;
+			case SIG('P', 'X'):
+				inode->i_mode = isonum_733(rr->u.PX.mode);
+				inode->i_nlink = isonum_733(rr->u.PX.n_links);
+				inode->i_uid = isonum_733(rr->u.PX.uid);
+				inode->i_gid = isonum_733(rr->u.PX.gid);
+				break;
+			case SIG('P', 'N'):
+				{
+					int high, low;
+					high = isonum_733(rr->u.PN.dev_high);
+					low = isonum_733(rr->u.PN.dev_low);
+					/*
+					 * The Rock Ridge standard specifies that if sizeof(dev_t) <= 4,
+					 * then the high field is unused, and the device number is completely
+					 * stored in the low field.  Some writers may ignore this subtlety,
+					 * and as a result we test to see if the entire device number is
+					 * stored in the low field, and use that.
+					 */
+					if ((low & ~0xff) && high == 0) {
+						inode->i_rdev =
+						    MKDEV(low >> 8, low & 0xff);
+					} else {
+						inode->i_rdev =
+						    MKDEV(high, low);
+					}
+				}
+				break;
+			case SIG('T', 'F'):
+				/* Some RRIP writers incorrectly place ctime in the TF_CREATE field.
+				   Try to handle this correctly for either case. */
+				cnt = 0;	/* Rock ridge never appears on a High Sierra disk */
+				if (rr->u.TF.flags & TF_CREATE) {
+					inode->i_ctime.tv_sec =
+					    iso_date(rr->u.TF.times[cnt++].time,
+						     0);
+					inode->i_ctime.tv_nsec = 0;
+				}
+				if (rr->u.TF.flags & TF_MODIFY) {
+					inode->i_mtime.tv_sec =
+					    iso_date(rr->u.TF.times[cnt++].time,
+						     0);
+					inode->i_mtime.tv_nsec = 0;
+				}
+				if (rr->u.TF.flags & TF_ACCESS) {
+					inode->i_atime.tv_sec =
+					    iso_date(rr->u.TF.times[cnt++].time,
+						     0);
+					inode->i_atime.tv_nsec = 0;
+				}
+				if (rr->u.TF.flags & TF_ATTRIBUTES) {
+					inode->i_ctime.tv_sec =
+					    iso_date(rr->u.TF.times[cnt++].time,
+						     0);
+					inode->i_ctime.tv_nsec = 0;
+				}
+				break;
+			case SIG('S', 'L'):
+				{
+					int slen;
+					struct SL_component *slp;
+					struct SL_component *oldslp;
+					slen = rr->len - 5;
+					slp = &rr->u.SL.link;
+					inode->i_size = symlink_len;
+					while (slen > 1) {
+						rootflag = 0;
+						switch (slp->flags & ~1) {
+						case 0:
+							inode->i_size +=
+							    slp->len;
+							break;
+						case 2:
+							inode->i_size += 1;
+							break;
+						case 4:
+							inode->i_size += 2;
+							break;
+						case 8:
+							rootflag = 1;
+							inode->i_size += 1;
+							break;
+						default:
+							printk
+							    ("Symlink component flag not implemented\n");
+						}
+						slen -= slp->len + 2;
+						oldslp = slp;
+						slp =
+						    (struct SL_component
+						     *)(((char *)slp) +
+							slp->len + 2);
+
+						if (slen < 2) {
+							if (((rr->u.SL.
+							      flags & 1) != 0)
+							    &&
+							    ((oldslp->
+							      flags & 1) == 0))
+								inode->i_size +=
+								    1;
+							break;
+						}
+
+						/*
+						 * If this component record isn't continued, then append a '/'.
+						 */
+						if (!rootflag
+						    && (oldslp->flags & 1) == 0)
+							inode->i_size += 1;
+					}
+				}
+				symlink_len = inode->i_size;
+				break;
+			case SIG('R', 'E'):
+				printk(KERN_WARNING
+				       "Attempt to read inode for relocated directory\n");
+				goto out;
+			case SIG('C', 'L'):
+				ISOFS_I(inode)->i_first_extent =
+				    isonum_733(rr->u.CL.location);
+				reloc =
+				    isofs_iget(inode->i_sb,
+					       ISOFS_I(inode)->i_first_extent,
+					       0);
+				if (!reloc)
+					goto out;
+				inode->i_mode = reloc->i_mode;
+				inode->i_nlink = reloc->i_nlink;
+				inode->i_uid = reloc->i_uid;
+				inode->i_gid = reloc->i_gid;
+				inode->i_rdev = reloc->i_rdev;
+				inode->i_size = reloc->i_size;
+				inode->i_blocks = reloc->i_blocks;
+				inode->i_atime = reloc->i_atime;
+				inode->i_ctime = reloc->i_ctime;
+				inode->i_mtime = reloc->i_mtime;
+				iput(reloc);
+				break;
 #ifdef CONFIG_ZISOFS
-      case SIG('Z','F'):
-	      if ( !ISOFS_SB(inode->i_sb)->s_nocompress ) {
-		      int algo;
-		      algo = isonum_721(rr->u.ZF.algorithm);
-		      if ( algo == SIG('p','z') ) {
-			      int block_shift = isonum_711(&rr->u.ZF.parms[1]);
-			      if ( block_shift < PAGE_CACHE_SHIFT || block_shift > 17 ) {
-				      printk(KERN_WARNING "isofs: Can't handle ZF block size of 2^%d\n", block_shift);
-			      } else {
-				/* Note: we don't change i_blocks here */
-				      ISOFS_I(inode)->i_file_format = isofs_file_compressed;
-				/* Parameters to compression algorithm (header size, block size) */
-				      ISOFS_I(inode)->i_format_parm[0] = isonum_711(&rr->u.ZF.parms[0]);
-				      ISOFS_I(inode)->i_format_parm[1] = isonum_711(&rr->u.ZF.parms[1]);
-				      inode->i_size = isonum_733(rr->u.ZF.real_size);
-			      }
-		      } else {
-			      printk(KERN_WARNING "isofs: Unknown ZF compression algorithm: %c%c\n",
-				     rr->u.ZF.algorithm[0], rr->u.ZF.algorithm[1]);
-		      }
-	      }
-	      break;
+			case SIG('Z', 'F'):
+				if (!ISOFS_SB(inode->i_sb)->s_nocompress) {
+					int algo;
+					algo = isonum_721(rr->u.ZF.algorithm);
+					if (algo == SIG('p', 'z')) {
+						int block_shift =
+						    isonum_711(&rr->u.ZF.
+							       parms[1]);
+						if (block_shift <
+						    PAGE_CACHE_SHIFT
+						    || block_shift > 17) {
+							printk(KERN_WARNING
+							       "isofs: Can't handle ZF block size of 2^%d\n",
+							       block_shift);
+						} else {
+							/* Note: we don't change i_blocks here */
+							ISOFS_I(inode)->
+							    i_file_format =
+							    isofs_file_compressed;
+							/* Parameters to compression algorithm (header size, block size) */
+							ISOFS_I(inode)->
+							    i_format_parm[0] =
+							    isonum_711(&rr->u.
+								       ZF.
+								       parms
+								       [0]);
+							ISOFS_I(inode)->
+							    i_format_parm[1] =
+							    isonum_711(&rr->u.
+								       ZF.
+								       parms
+								       [1]);
+							inode->i_size =
+							    isonum_733(rr->u.ZF.
+								       real_size);
+						}
+					} else {
+						printk(KERN_WARNING
+						       "isofs: Unknown ZF compression algorithm: %c%c\n",
+						       rr->u.ZF.algorithm[0],
+						       rr->u.ZF.algorithm[1]);
+					}
+				}
+				break;
 #endif
-      default:
-	break;
-      }
-    }
-  }
-  MAYBE_CONTINUE(repeat,inode);
- out:
-  if(buffer) kfree(buffer);
-  return 0;
+			default:
+				break;
+			}
+		}
+	}
+	MAYBE_CONTINUE(repeat, inode);
+      out:
+	if (buffer)
+		kfree(buffer);
+	return 0;
 }
 
 static char *get_symlink_chunk(char *rpnt, struct rock_ridge *rr, char *plimit)
@@ -382,32 +449,32 @@
 			if (slp->len > plimit - rpnt)
 				return NULL;
 			memcpy(rpnt, slp->text, slp->len);
-			rpnt+=slp->len;
+			rpnt += slp->len;
 			break;
 		case 2:
 			if (rpnt >= plimit)
 				return NULL;
-			*rpnt++='.';
+			*rpnt++ = '.';
 			break;
 		case 4:
 			if (2 > plimit - rpnt)
 				return NULL;
-			*rpnt++='.';
-			*rpnt++='.';
+			*rpnt++ = '.';
+			*rpnt++ = '.';
 			break;
 		case 8:
 			if (rpnt >= plimit)
 				return NULL;
 			rootflag = 1;
-			*rpnt++='/';
+			*rpnt++ = '/';
 			break;
 		default:
 			printk("Symlink component flag not implemented (%d)\n",
-			     slp->flags);
+			       slp->flags);
 		}
 		slen -= slp->len + 2;
 		oldslp = slp;
-		slp = (struct SL_component *) ((char *) slp + slp->len + 2);
+		slp = (struct SL_component *)((char *)slp + slp->len + 2);
 
 		if (slen < 2) {
 			/*
@@ -418,7 +485,7 @@
 			    !(oldslp->flags & 1)) {
 				if (rpnt >= plimit)
 					return NULL;
-				*rpnt++='/';
+				*rpnt++ = '/';
 			}
 			break;
 		}
@@ -429,24 +496,22 @@
 		if (!rootflag && !(oldslp->flags & 1)) {
 			if (rpnt >= plimit)
 				return NULL;
-			*rpnt++='/';
+			*rpnt++ = '/';
 		}
 	}
 	return rpnt;
 }
 
-int parse_rock_ridge_inode(struct iso_directory_record * de,
-			   struct inode * inode)
+int parse_rock_ridge_inode(struct iso_directory_record *de, struct inode *inode)
 {
-   int result=parse_rock_ridge_inode_internal(de,inode,0);
-   /* if rockridge flag was reset and we didn't look for attributes
-    * behind eventual XA attributes, have a look there */
-   if ((ISOFS_SB(inode->i_sb)->s_rock_offset==-1)
-       &&(ISOFS_SB(inode->i_sb)->s_rock==2))
-     {
-	result=parse_rock_ridge_inode_internal(de,inode,14);
-     }
-   return result;
+	int result = parse_rock_ridge_inode_internal(de, inode, 0);
+	/* if rockridge flag was reset and we didn't look for attributes
+	 * behind eventual XA attributes, have a look there */
+	if ((ISOFS_SB(inode->i_sb)->s_rock_offset == -1)
+	    && (ISOFS_SB(inode->i_sb)->s_rock == 2)) {
+		result = parse_rock_ridge_inode_internal(de, inode, 14);
+	}
+	return result;
 }
 
 /* readpage() for symlinks: reads symlink contents into the page and either
@@ -455,7 +520,7 @@
 static int rock_ridge_symlink_readpage(struct file *file, struct page *page)
 {
 	struct inode *inode = page->mapping->host;
-        struct iso_inode_info *ei = ISOFS_I(inode);
+	struct iso_inode_info *ei = ISOFS_I(inode);
 	char *link = kmap(page);
 	unsigned long bufsize = ISOFS_BUFFER_SIZE(inode);
 	struct buffer_head *bh;
@@ -478,10 +543,10 @@
 	if (!bh)
 		goto out_noread;
 
-        offset = ei->i_iget5_offset;
-	pnt = (unsigned char *) bh->b_data + offset;
+	offset = ei->i_iget5_offset;
+	pnt = (unsigned char *)bh->b_data + offset;
 
-	raw_inode = (struct iso_directory_record *) pnt;
+	raw_inode = (struct iso_directory_record *)pnt;
 
 	/*
 	 * If we go past the end of the buffer, there is some sort of error.
@@ -495,8 +560,8 @@
 	SETUP_ROCK_RIDGE(raw_inode, chr, len);
 
       repeat:
-	while (len > 2) { /* There may be one byte for padding somewhere */
-		rr = (struct rock_ridge *) chr;
+	while (len > 2) {	/* There may be one byte for padding somewhere */
+		rr = (struct rock_ridge *)chr;
 		if (rr->len < 3)
 			goto out;	/* Something got screwed up here */
 		sig = isonum_721(chr);
@@ -561,5 +626,5 @@
 }
 
 struct address_space_operations isofs_symlink_aops = {
-	.readpage	= rock_ridge_symlink_readpage
+	.readpage = rock_ridge_symlink_readpage
 };

[PATCH 2/9] isofs: inline macros in rock.c

[Pekka Enberg]

This patch inlines the CONTINUE_DECLS macro in fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:27:39.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:27:40.000000000 +0300
@@ -38,10 +38,6 @@
    same thing in certain places.  We use the macros to ensure that everything
    is done correctly */
 
-#define CONTINUE_DECLS \
-  int cont_extent = 0, cont_offset = 0, cont_size = 0;   \
-  void *buffer = NULL
-
 #define CHECK_CE	       			\
       {cont_extent = isonum_733(rr->u.CE.extent); \
       cont_offset = isonum_733(rr->u.CE.offset); \
@@ -95,7 +91,8 @@
 {
 	int len;
 	unsigned char *chr;
-	CONTINUE_DECLS;
+	int cont_extent = 0, cont_offset = 0, cont_size = 0;
+	void *buffer = NULL;
 	int retnamlen = 0, truncate = 0;
 
 	if (!ISOFS_SB(inode->i_sb)->s_rock)
@@ -184,7 +181,8 @@
 	int len;
 	unsigned char *chr;
 	int symlink_len = 0;
-	CONTINUE_DECLS;
+	int cont_extent = 0, cont_offset = 0, cont_size = 0;
+	void *buffer = NULL;
 
 	if (!ISOFS_SB(inode->i_sb)->s_rock)
 		return 0;
@@ -527,7 +525,8 @@
 	char *rpnt = link;
 	unsigned char *pnt;
 	struct iso_directory_record *raw_inode;
-	CONTINUE_DECLS;
+	int cont_extent = 0, cont_offset = 0, cont_size = 0;
+	void *buffer = NULL;
 	unsigned long block, offset;
 	int sig;
 	int len;

[PATCH 3/9] isofs: inline macros in rock.c

[Pekka Enberg]

This patch inlines the CHECK_CE macro in fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:27:40.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:27:47.000000000 +0300
@@ -38,11 +38,6 @@
    same thing in certain places.  We use the macros to ensure that everything
    is done correctly */
 
-#define CHECK_CE	       			\
-      {cont_extent = isonum_733(rr->u.CE.extent); \
-      cont_offset = isonum_733(rr->u.CE.offset); \
-      cont_size = isonum_733(rr->u.CE.size);}
-
 #define SETUP_ROCK_RIDGE(DE,CHR,LEN)	      		      	\
   {LEN= sizeof(struct iso_directory_record) + DE->name_len[0];	\
   if(LEN & 1) LEN++;						\
@@ -124,7 +119,11 @@
 				CHECK_SP(goto out);
 				break;
 			case SIG('C', 'E'):
-				CHECK_CE;
+				{
+					cont_extent = isonum_733(rr->u.CE.extent);
+					cont_offset = isonum_733(rr->u.CE.offset);
+					cont_size = isonum_733(rr->u.CE.size);
+				}
 				break;
 			case SIG('N', 'M'):
 				if (truncate)
@@ -224,7 +223,11 @@
 				CHECK_SP(goto out);
 				break;
 			case SIG('C', 'E'):
-				CHECK_CE;
+				{
+					cont_extent = isonum_733(rr->u.CE.extent);
+					cont_offset = isonum_733(rr->u.CE.offset);
+					cont_size = isonum_733(rr->u.CE.size);
+				}
 				break;
 			case SIG('E', 'R'):
 				ISOFS_SB(inode->i_sb)->s_rock = 1;
@@ -585,7 +588,11 @@
 			break;
 		case SIG('C', 'E'):
 			/* This tells is if there is a continuation record */
-			CHECK_CE;
+			{
+				cont_extent = isonum_733(rr->u.CE.extent);
+				cont_offset = isonum_733(rr->u.CE.offset);
+				cont_size = isonum_733(rr->u.CE.size);
+			}
 		default:
 			break;
 		}

[PATCH 4/9] isofs: inline macros in rock.c

[Pekka Enberg]

This patch inlines the MAYBE_CONTINUE macro in fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |  121 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 90 insertions(+), 31 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:28:05.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:31:53.000000000 +0300
@@ -52,34 +52,6 @@
   }                                                             \
 }
 
-#define MAYBE_CONTINUE(LABEL,DEV) \
-  {if (buffer) { kfree(buffer); buffer = NULL; } \
-  if (cont_extent){ \
-    int block, offset, offset1; \
-    struct buffer_head * pbh; \
-    buffer = kmalloc(cont_size,GFP_KERNEL); \
-    if (!buffer) goto out; \
-    block = cont_extent; \
-    offset = cont_offset; \
-    offset1 = 0; \
-    pbh = sb_bread(DEV->i_sb, block); \
-    if(pbh){       \
-      if (offset > pbh->b_size || offset + cont_size > pbh->b_size){	\
-	brelse(pbh); \
-	goto out; \
-      } \
-      memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1); \
-      brelse(pbh); \
-      chr = (unsigned char *) buffer; \
-      len = cont_size; \
-      cont_extent = 0; \
-      cont_size = 0; \
-      cont_offset = 0; \
-      goto LABEL; \
-    }    \
-    printk("Unable to read rock-ridge attributes\n");    \
-  }}
-
 /* return length of name field; 0: not found, -1: to be ignored */
 int get_rock_ridge_filename(struct iso_directory_record *de,
 			    char *retname, struct inode *inode)
@@ -163,7 +135,36 @@
 			}
 		}
 	}
-	MAYBE_CONTINUE(repeat, inode);
+	if (buffer) {
+		kfree(buffer);
+		buffer = NULL;
+	}
+	if (cont_extent) {
+		int block, offset, offset1;
+		struct buffer_head * pbh;
+		buffer = kmalloc(cont_size,GFP_KERNEL);
+		if (!buffer)
+			goto out;
+		block = cont_extent;
+		offset = cont_offset;
+		offset1 = 0;
+		pbh = sb_bread(inode->i_sb, block);
+		if(pbh) {
+			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+				brelse(pbh);
+				goto out;
+     			}
+			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			brelse(pbh);
+			chr = (unsigned char *) buffer;
+			len = cont_size;
+			cont_extent = 0;
+			cont_size = 0;
+			cont_offset = 0;
+			goto repeat;
+		} 
+		printk("Unable to read rock-ridge attributes\n");
+	}
 	if (buffer)
 		kfree(buffer);
 	return retnamlen;	/* If 0, this file did not have a NM field */
@@ -428,7 +429,36 @@
 			}
 		}
 	}
-	MAYBE_CONTINUE(repeat, inode);
+	if (buffer) {
+		kfree(buffer);
+		buffer = NULL;
+	}
+	if (cont_extent) {
+		int block, offset, offset1;
+		struct buffer_head * pbh;
+		buffer = kmalloc(cont_size,GFP_KERNEL);
+		if (!buffer)
+			goto out;
+		block = cont_extent;
+		offset = cont_offset;
+		offset1 = 0;
+		pbh = sb_bread(inode->i_sb, block);
+		if(pbh) {
+			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+				brelse(pbh);
+				goto out;
+     			}
+			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			brelse(pbh);
+			chr = (unsigned char *) buffer;
+			len = cont_size;
+			cont_extent = 0;
+			cont_size = 0;
+			cont_offset = 0;
+			goto repeat;
+		} 
+		printk("Unable to read rock-ridge attributes\n");
+	}
       out:
 	if (buffer)
 		kfree(buffer);
@@ -597,7 +627,36 @@
 			break;
 		}
 	}
-	MAYBE_CONTINUE(repeat, inode);
+	if (buffer) {
+		kfree(buffer);
+		buffer = NULL;
+	}
+	if (cont_extent) {
+		int block, offset, offset1;
+		struct buffer_head * pbh;
+		buffer = kmalloc(cont_size,GFP_KERNEL);
+		if (!buffer)
+			goto out;
+		block = cont_extent;
+		offset = cont_offset;
+		offset1 = 0;
+		pbh = sb_bread(inode->i_sb, block);
+		if(pbh) {
+			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+				brelse(pbh);
+				goto out;
+     			}
+			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			brelse(pbh);
+			chr = (unsigned char *) buffer;
+			len = cont_size;
+			cont_extent = 0;
+			cont_size = 0;
+			cont_offset = 0;
+			goto repeat;
+		} 
+		printk("Unable to read rock-ridge attributes\n");
+	}
 	if (buffer)
 		kfree(buffer);
 

[PATCH 5/9] isofs: convert macro to function in rock.c

[Pekka Enberg]

This patch converts the SETUP_ROCK_RIDGE macro to a proper function in
fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:31:53.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:32:13.000000000 +0300
@@ -38,18 +38,22 @@
    same thing in certain places.  We use the macros to ensure that everything
    is done correctly */
 
-#define SETUP_ROCK_RIDGE(DE,CHR,LEN)	      		      	\
-  {LEN= sizeof(struct iso_directory_record) + DE->name_len[0];	\
-  if(LEN & 1) LEN++;						\
-  CHR = ((unsigned char *) DE) + LEN;				\
-  LEN = *((unsigned char *) DE) - LEN;                          \
-  if (LEN<0) LEN=0;                                             \
-  if (ISOFS_SB(inode->i_sb)->s_rock_offset!=-1)                \
-  {                                                             \
-     LEN-=ISOFS_SB(inode->i_sb)->s_rock_offset;                \
-     CHR+=ISOFS_SB(inode->i_sb)->s_rock_offset;                \
-     if (LEN<0) LEN=0;                                          \
-  }                                                             \
+static int setup_rock_ridge(struct iso_directory_record *de, struct inode *inode, unsigned char ** chr)
+{
+	int len = sizeof(struct iso_directory_record) + de->name_len[0];
+	if (len & 1)
+		len++;
+	*chr = ((unsigned char *) de) + len;
+	len = *((unsigned char *) de) - len;
+	if (len < 0)
+		len = 0;
+	if (ISOFS_SB(inode->i_sb)->s_rock_offset != -1) {
+		len -= ISOFS_SB(inode->i_sb)->s_rock_offset;
+		*chr += ISOFS_SB(inode->i_sb)->s_rock_offset;
+		if (len < 0)
+			len = 0;
+	}
+	return len;
 }
 
 /* return length of name field; 0: not found, -1: to be ignored */
@@ -66,7 +70,7 @@
 		return 0;
 	*retname = 0;
 
-	SETUP_ROCK_RIDGE(de, chr, len);
+	len = setup_rock_ridge(de, inode, &chr);
       repeat:
 	{
 		struct rock_ridge *rr;
@@ -187,7 +191,7 @@
 	if (!ISOFS_SB(inode->i_sb)->s_rock)
 		return 0;
 
-	SETUP_ROCK_RIDGE(de, chr, len);
+	len = setup_rock_ridge(de, inode, &chr);
 	if (regard_xa) {
 		chr += 14;
 		len -= 14;
@@ -589,7 +593,7 @@
 	/* Now test for possible Rock Ridge extensions which will override
 	   some of these numbers in the inode structure. */
 
-	SETUP_ROCK_RIDGE(raw_inode, chr, len);
+	len = setup_rock_ridge(raw_inode, inode, &chr);
 
       repeat:
 	while (len > 2) {	/* There may be one byte for padding somewhere */

[PATCH 6/9] isofs: convert macro to function in rock.c

[Pekka Enberg]

This patch converts the CHECK_SP macro to a proper function in
fs/isofs/rock.c. 

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 16:32:13.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 16:32:16.000000000 +0300
@@ -28,15 +28,13 @@
 
 #define SIG(A,B) ((A) | ((B) << 8))	/* isonum_721() */
 
-/* This is a way of ensuring that we have something in the system
-   use fields that is compatible with Rock Ridge */
-#define CHECK_SP(FAIL)	       			\
-      if(rr->u.SP.magic[0] != 0xbe) FAIL;	\
-      if(rr->u.SP.magic[1] != 0xef) FAIL;       \
-      ISOFS_SB(inode->i_sb)->s_rock_offset=rr->u.SP.skip;
-/* We define a series of macros because each function must do exactly the
-   same thing in certain places.  We use the macros to ensure that everything
-   is done correctly */
+static inline int rock_set_offset(struct inode *inode, struct rock_ridge * rr)
+{
+      if (rr->u.SP.magic[0] != 0xbe || rr->u.SP.magic[1] != 0xef)
+	      return 0;
+      ISOFS_SB(inode->i_sb)->s_rock_offset = rr->u.SP.skip;
+      return 1;
+}
 
 static int setup_rock_ridge(struct iso_directory_record *de, struct inode *inode, unsigned char ** chr)
 {
@@ -92,7 +90,8 @@
 					goto out;
 				break;
 			case SIG('S', 'P'):
-				CHECK_SP(goto out);
+				if (!rock_set_offset(inode, rr))
+					goto out;
 				break;
 			case SIG('C', 'E'):
 				{
@@ -225,7 +224,8 @@
 				break;
 #endif
 			case SIG('S', 'P'):
-				CHECK_SP(goto out);
+				if (!rock_set_offset(inode, rr))
+					goto out;
 				break;
 			case SIG('C', 'E'):
 				{
@@ -612,7 +612,8 @@
 				goto out;
 			break;
 		case SIG('S', 'P'):
-			CHECK_SP(goto out);
+			if (!rock_set_offset(inode, rr))
+				goto out;
 			break;
 		case SIG('S', 'L'):
 			rpnt = get_symlink_chunk(rpnt, rr,

[PATCH 7/9] isofs: extract zisofs parsing to function

[Pekka Enberg]

This patch extracts ZISOFS entry parsing to a separate function.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   77 +++++++++++++++++++++++++++++------------------------------------
 1 files changed, 35 insertions(+), 42 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 18:26:34.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 18:47:40.000000000 +0300
@@ -177,6 +177,40 @@
 	return 0;
 }
 
+#ifdef CONFIG_ZISOFS
+static void rock_parse_zisofs_entry(struct rock_ridge *rr, struct inode *inode)
+{
+	int algo;
+
+	if (ISOFS_SB(inode->i_sb)->s_nocompress)
+		return;
+
+	algo = isonum_721(rr->u.ZF.algorithm);
+	if (algo == SIG('p', 'z')) {
+		int block_shift = isonum_711(&rr->u.ZF.parms[1]);
+		if (block_shift < PAGE_CACHE_SHIFT || block_shift > 17) {
+			printk(KERN_WARNING
+			       "isofs: Can't handle ZF block size of 2^%d\n",
+			       block_shift);
+		} else {
+			/* Note: we don't change i_blocks here */
+			ISOFS_I(inode)->i_file_format = isofs_file_compressed;
+			/* Parameters to compression algorithm (header size, block size) */
+			ISOFS_I(inode)->i_format_parm[0] =
+				isonum_711(&rr->u.ZF.parms[0]);
+			ISOFS_I(inode)->i_format_parm[1] =
+				isonum_711(&rr->u.ZF.parms[1]);
+			inode->i_size = isonum_733(rr->u.ZF.real_size);
+		}
+	} else {
+		printk(KERN_WARNING
+		       "isofs: Unknown ZF compression algorithm: %c%c\n",
+		       rr->u.ZF.algorithm[0],
+		       rr->u.ZF.algorithm[1]);
+	}
+}
+#endif
+
 static int
 parse_rock_ridge_inode_internal(struct iso_directory_record *de,
 				struct inode *inode, int regard_xa)
@@ -384,48 +418,7 @@
 				break;
 #ifdef CONFIG_ZISOFS
 			case SIG('Z', 'F'):
-				if (!ISOFS_SB(inode->i_sb)->s_nocompress) {
-					int algo;
-					algo = isonum_721(rr->u.ZF.algorithm);
-					if (algo == SIG('p', 'z')) {
-						int block_shift =
-						    isonum_711(&rr->u.ZF.
-							       parms[1]);
-						if (block_shift <
-						    PAGE_CACHE_SHIFT
-						    || block_shift > 17) {
-							printk(KERN_WARNING
-							       "isofs: Can't handle ZF block size of 2^%d\n",
-							       block_shift);
-						} else {
-							/* Note: we don't change i_blocks here */
-							ISOFS_I(inode)->
-							    i_file_format =
-							    isofs_file_compressed;
-							/* Parameters to compression algorithm (header size, block size) */
-							ISOFS_I(inode)->
-							    i_format_parm[0] =
-							    isonum_711(&rr->u.
-								       ZF.
-								       parms
-								       [0]);
-							ISOFS_I(inode)->
-							    i_format_parm[1] =
-							    isonum_711(&rr->u.
-								       ZF.
-								       parms
-								       [1]);
-							inode->i_size =
-							    isonum_733(rr->u.ZF.
-								       real_size);
-						}
-					} else {
-						printk(KERN_WARNING
-						       "isofs: Unknown ZF compression algorithm: %c%c\n",
-						       rr->u.ZF.algorithm[0],
-						       rr->u.ZF.algorithm[1]);
-					}
-				}
+				rock_parse_zisofs_entry(rr, inode);
 				break;
 #endif
 			default:

[PATCH 8/9] isofs: remove redundant kfree checks from rock

[Pekka Enberg]

This patch removes redundant kfree() NULL checks from fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   36 ++++++++++++------------------------
 1 files changed, 12 insertions(+), 24 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 18:24:50.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 18:24:53.000000000 +0300
@@ -130,18 +130,15 @@
 				retnamlen += rr->len - 5;
 				break;
 			case SIG('R', 'E'):
-				if (buffer)
-					kfree(buffer);
+				kfree(buffer);
 				return -1;
 			default:
 				break;
 			}
 		}
 	}
-	if (buffer) {
-		kfree(buffer);
-		buffer = NULL;
-	}
+	kfree(buffer);
+	buffer = NULL;
 	if (cont_extent) {
 		int block, offset, offset1;
 		struct buffer_head * pbh;
@@ -168,12 +165,10 @@
 		} 
 		printk("Unable to read rock-ridge attributes\n");
 	}
-	if (buffer)
-		kfree(buffer);
+	kfree(buffer);
 	return retnamlen;	/* If 0, this file did not have a NM field */
       out:
-	if (buffer)
-		kfree(buffer);
+	kfree(buffer);
 	return 0;
 }
 
@@ -426,10 +421,8 @@
 			}
 		}
 	}
-	if (buffer) {
-		kfree(buffer);
-		buffer = NULL;
-	}
+	kfree(buffer);
+	buffer = NULL;
 	if (cont_extent) {
 		int block, offset, offset1;
 		struct buffer_head * pbh;
@@ -457,8 +450,7 @@
 		printk("Unable to read rock-ridge attributes\n");
 	}
       out:
-	if (buffer)
-		kfree(buffer);
+	kfree(buffer);
 	return 0;
 }
 
@@ -625,10 +617,8 @@
 			break;
 		}
 	}
-	if (buffer) {
-		kfree(buffer);
-		buffer = NULL;
-	}
+	kfree(buffer);
+	buffer = NULL;
 	if (cont_extent) {
 		int block, offset, offset1;
 		struct buffer_head * pbh;
@@ -655,8 +645,7 @@
 		} 
 		printk("Unable to read rock-ridge attributes\n");
 	}
-	if (buffer)
-		kfree(buffer);
+	kfree(buffer);
 
 	if (rpnt == link)
 		goto fail;
@@ -670,8 +659,7 @@
 
 	/* error exit from macro */
       out:
-	if (buffer)
-		kfree(buffer);
+	kfree(buffer);
 	goto fail;
       out_noread:
 	printk("unable to read i-node block");

[PATCH 9/9] isofs: clean up rock.c

[Pekka Enberg]

This patch removes some redundant variables from fs/isofs/rock.c.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 rock.c |   41 +++++++++++++----------------------------
 1 files changed, 13 insertions(+), 28 deletions(-)

Index: 2.6/fs/isofs/rock.c
===================================================================
--- 2.6.orig/fs/isofs/rock.c	2005-03-28 18:26:34.000000000 +0300
+++ 2.6/fs/isofs/rock.c	2005-03-28 18:27:35.000000000 +0300
@@ -140,21 +140,17 @@
 	kfree(buffer);
 	buffer = NULL;
 	if (cont_extent) {
-		int block, offset, offset1;
 		struct buffer_head * pbh;
 		buffer = kmalloc(cont_size,GFP_KERNEL);
 		if (!buffer)
 			goto out;
-		block = cont_extent;
-		offset = cont_offset;
-		offset1 = 0;
-		pbh = sb_bread(inode->i_sb, block);
+		pbh = sb_bread(inode->i_sb, cont_extent);
 		if(pbh) {
-			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+			if (cont_offset > pbh->b_size || cont_offset + cont_size > pbh->b_size) {
 				brelse(pbh);
 				goto out;
      			}
-			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			memcpy(buffer, pbh->b_data + cont_offset, cont_size);
 			brelse(pbh);
 			chr = (unsigned char *) buffer;
 			len = cont_size;
@@ -424,21 +420,17 @@
 	kfree(buffer);
 	buffer = NULL;
 	if (cont_extent) {
-		int block, offset, offset1;
 		struct buffer_head * pbh;
 		buffer = kmalloc(cont_size,GFP_KERNEL);
 		if (!buffer)
 			goto out;
-		block = cont_extent;
-		offset = cont_offset;
-		offset1 = 0;
-		pbh = sb_bread(inode->i_sb, block);
+		pbh = sb_bread(inode->i_sb, cont_extent);
 		if(pbh) {
-			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+			if (cont_offset > pbh->b_size || cont_offset + cont_size > pbh->b_size) {
 				brelse(pbh);
 				goto out;
      			}
-			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			memcpy(buffer, pbh->b_data + cont_offset, cont_size);
 			brelse(pbh);
 			chr = (unsigned char *) buffer;
 			len = cont_size;
@@ -549,8 +541,6 @@
 	struct iso_directory_record *raw_inode;
 	int cont_extent = 0, cont_offset = 0, cont_size = 0;
 	void *buffer = NULL;
-	unsigned long block, offset;
-	int sig;
 	int len;
 	unsigned char *chr;
 	struct rock_ridge *rr;
@@ -558,21 +548,19 @@
 	if (!ISOFS_SB(inode->i_sb)->s_rock)
 		goto error;
 
-	block = ei->i_iget5_block;
 	lock_kernel();
-	bh = sb_bread(inode->i_sb, block);
+	bh = sb_bread(inode->i_sb, ei->i_iget5_block);
 	if (!bh)
 		goto out_noread;
 
-	offset = ei->i_iget5_offset;
-	pnt = (unsigned char *)bh->b_data + offset;
+	pnt = (unsigned char *)bh->b_data + ei->i_iget5_offset;
 
 	raw_inode = (struct iso_directory_record *)pnt;
 
 	/*
 	 * If we go past the end of the buffer, there is some sort of error.
 	 */
-	if (offset + *pnt > bufsize)
+	if (ei->i_iget5_offset + *pnt > bufsize)
 		goto out_bad_span;
 
 	/* Now test for possible Rock Ridge extensions which will override
@@ -582,6 +570,7 @@
 
       repeat:
 	while (len > 2) {	/* There may be one byte for padding somewhere */
+		int sig;
 		rr = (struct rock_ridge *)chr;
 		if (rr->len < 3)
 			goto out;	/* Something got screwed up here */
@@ -620,21 +609,17 @@
 	kfree(buffer);
 	buffer = NULL;
 	if (cont_extent) {
-		int block, offset, offset1;
 		struct buffer_head * pbh;
 		buffer = kmalloc(cont_size,GFP_KERNEL);
 		if (!buffer)
 			goto out;
-		block = cont_extent;
-		offset = cont_offset;
-		offset1 = 0;
-		pbh = sb_bread(inode->i_sb, block);
+		pbh = sb_bread(inode->i_sb, cont_extent);
 		if(pbh) {
-			if (offset > pbh->b_size || offset + cont_size > pbh->b_size) {
+			if (cont_offset > pbh->b_size || cont_offset + cont_size > pbh->b_size) {
 				brelse(pbh);
 				goto out;
      			}
-			memcpy(buffer + offset1, pbh->b_data + offset, cont_size - offset1);
+			memcpy(buffer, pbh->b_data + cont_offset, cont_size);
 			brelse(pbh);
 			chr = (unsigned char *) buffer;
 			len = cont_size;

Re: [PATCH 0/9] isofs: unobfuscate rock.c

[Chris Wright]

* Pekka Enberg (penberg@cs.helsinki.fi) wrote:
> This patch removes macro obfuscation from fs/isofs/rock.c and cleans it up
> a bit to make it more readable and maintainable. There are no functional
> changes, only cleanups. I have only tested this lightly but it passes
> mount and read on small Rock Ridge enabled ISO image.

You might want to look at current -mm.  Andrew has a series or 13 or so
patches that do very similar cleanup.  Perhaps you could start from there?

thanks,
-chris

Re: [PATCH 0/9] isofs: unobfuscate rock.c

[Andries Brouwer]

On Mon, Mar 28, 2005 at 12:02:52PM -0800, Chris Wright wrote:
> * Pekka Enberg (penberg@cs.helsinki.fi) wrote:
> > This patch removes macro obfuscation from fs/isofs/rock.c and cleans it up
> > a bit to make it more readable and maintainable. There are no functional
> > changes, only cleanups. I have only tested this lightly but it passes
> > mount and read on small Rock Ridge enabled ISO image.
> 
> You might want to look at current -mm.  Andrew has a series or 13 or so
> patches that do very similar cleanup.  Perhaps you could start from there?

Good! When Linus asked I audited rock.c and also did rather similar polishing -
it happens automatically if one looks at this code. But it seems everybody is
doing this right now, so I must wait a few weeks and see what got into Linus'
tree. Linus plugged many but not all holes. (Maybe you did more?)

Andries

Re: [PATCH 0/9] isofs: unobfuscate rock.c

[Chris Wright]

* Andries Brouwer (aebr@win.tue.nl) wrote:
> On Mon, Mar 28, 2005 at 12:02:52PM -0800, Chris Wright wrote:
> > * Pekka Enberg (penberg@cs.helsinki.fi) wrote:
> > > This patch removes macro obfuscation from fs/isofs/rock.c and cleans it up
> > > a bit to make it more readable and maintainable. There are no functional
> > > changes, only cleanups. I have only tested this lightly but it passes
> > > mount and read on small Rock Ridge enabled ISO image.
> > 
> > You might want to look at current -mm.  Andrew has a series or 13 or so
> > patches that do very similar cleanup.  Perhaps you could start from there?
> 
> Good! When Linus asked I audited rock.c and also did rather similar polishing -
> it happens automatically if one looks at this code. But it seems everybody is
> doing this right now, so I must wait a few weeks and see what got into Linus'
> tree. Linus plugged many but not all holes. (Maybe you did more?)

I plugged one more (in 2.6.11.6 and in Linus' tree), and Andrew plugged
yet another (last patch in his iso/rock cleanup series).  More eyes
the better.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: isofs: unobfuscate rock.c

[Pekka J Enberg]

Hi Andries, 

Andries Brouwer writes:
> Good! When Linus asked I audited rock.c and also did rather similar polishing -
> it happens automatically if one looks at this code. But it seems everybody is
> doing this right now, so I must wait a few weeks and see what got into Linus'
> tree. Linus plugged many but not all holes. (Maybe you did more?)

I didn't fix anything. I tried to be very careful about preserving the 
current logic while making it more readable so I could find bugs :-). 

              Pekka