[PATCH] isofs/inode.c

[PATCH] isofs/inode.c

[Andries.Brouwer]

Here the second patch in the isofs series.
inode.c:isofs_read_super() dereferences the variable pri
that need not be set in case of a Joliet CD, causing an Oops.
Patch below.

Andries

[While editing the diff, I left a fix for aha1542.c,
maybe you got it already.  I also left something else
that always annoyed me: valuable screen space (on a 24x80 vt)
is lost by these silly [< >] around addresses in an Oops.
They provide no information at all, but on the other hand
cause loss of information because these lines no longer
fit in 80 columns causing line wrap and the loss of the
top of the Oops.]

diff -u --recursive --new-file ../linux-2.4.0-test11/linux/arch/i386/kernel/traps.c ./linux/arch/i386/kernel/traps.c
--- ../linux-2.4.0-test11/linux/arch/i386/kernel/traps.c	Tue Nov 21 21:44:05 2000
+++ ./linux/arch/i386/kernel/traps.c	Mon Nov 20 12:17:11 2000
@@ -146,7 +146,7 @@
 		    ((addr >= module_start) && (addr <= module_end))) {
 			if (i && ((i % 8) == 0))
 				printk("\n       ");
-			printk("[<%08lx>] ", addr);
+			printk("%08lx ", addr);	/* not [<...>] */
 			i++;
 		}
 	}
@@ -166,7 +166,7 @@
 		esp = regs->esp;
 		ss = regs->xss & 0xffff;
 	}
-	printk("CPU:    %d\nEIP:    %04x:[<%08lx>]\nEFLAGS: %08lx\n",
+	printk("CPU:    %d\nEIP:    %04x:%08lx\nEFLAGS: %08lx\n",
 		smp_processor_id(), 0xffff & regs->xcs, regs->eip, regs->eflags);
 	printk("eax: %08lx   ebx: %08lx   ecx: %08lx   edx: %08lx\n",
 		regs->eax, regs->ebx, regs->ecx, regs->edx);
diff -u --recursive --new-file ../linux-2.4.0-test11/linux/drivers/scsi/aha1542.c ./linux/drivers/scsi/aha1542.c
--- ../linux-2.4.0-test11/linux/drivers/scsi/aha1542.c	Tue Nov 21 21:44:10 2000
+++ ./linux/drivers/scsi/aha1542.c	Tue Nov 21 18:01:57 2000
@@ -1416,6 +1416,7 @@
 			SCtmp = HOSTDATA(SCpnt->host)->SCint[i];
 			if (SCtmp->host_scribble) {
 				scsi_free(SCtmp->host_scribble, 512);
+				SCtmp->host_scribble = NULL;
 			}
 			HOSTDATA(SCpnt->host)->SCint[i] = NULL;
 			HOSTDATA(SCpnt->host)->mb[i].status = 0;
@@ -1478,6 +1479,7 @@
 			}
 			if (SCtmp->host_scribble) {
 				scsi_free(SCtmp->host_scribble, 512);
+				SCtmp->host_scribble = NULL;
 			}
 			HOSTDATA(SCpnt->host)->SCint[i] = NULL;
 			HOSTDATA(SCpnt->host)->mb[i].status = 0;
@@ -1546,6 +1548,7 @@
 			}
 			if (SCtmp->host_scribble) {
 				scsi_free(SCtmp->host_scribble, 512);
+				SCtmp->host_scribble = NULL;
 			}
 			HOSTDATA(SCpnt->host)->SCint[i] = NULL;
 			HOSTDATA(SCpnt->host)->mb[i].status = 0;
@@ -1681,8 +1684,10 @@
 				Scsi_Cmnd *SCtmp;
 				SCtmp = HOSTDATA(SCpnt->host)->SCint[i];
 				SCtmp->result = DID_RESET << 16;
-				if (SCtmp->host_scribble)
+				if (SCtmp->host_scribble) {
 					scsi_free(SCtmp->host_scribble, 512);
+					SCtmp->host_scribble = NULL;
+				}
 				printk(KERN_WARNING "Sending DID_RESET for target %d\n", SCpnt->target);
 				SCtmp->scsi_done(SCpnt);
 
@@ -1725,8 +1730,10 @@
 						Scsi_Cmnd *SCtmp;
 						SCtmp = HOSTDATA(SCpnt->host)->SCint[i];
 						SCtmp->result = DID_RESET << 16;
-						if (SCtmp->host_scribble)
+						if (SCtmp->host_scribble) {
 							scsi_free(SCtmp->host_scribble, 512);
+							SCtmp->host_scribble = NULL;
+						}
 						printk(KERN_WARNING "Sending DID_RESET for target %d\n", SCpnt->target);
 						SCtmp->scsi_done(SCpnt);
 
diff -u --recursive --new-file ../linux-2.4.0-test11/linux/fs/isofs/inode.c ./linux/fs/isofs/inode.c
--- ../linux-2.4.0-test11/linux/fs/isofs/inode.c	Tue Nov 21 21:44:14 2000
+++ ./linux/fs/isofs/inode.c	Wed Nov 22 13:22:09 2000
@@ -500,15 +500,13 @@
  	 * that value.
  	 */
  	blocksize = get_hardblocksize(dev);
- 	if(    (blocksize != 0)
- 	    && (blocksize > opt.blocksize) )
- 	  {
+ 	if(blocksize > opt.blocksize) {
  	    /*
  	     * Force the blocksize we are going to use to be the
  	     * hardware blocksize.
  	     */
  	    opt.blocksize = blocksize;
- 	  }
+	}
  
 	blocksize_bits = 0;
 	{
@@ -594,6 +592,7 @@
 	    brelse(bh);
 	    bh = NULL;
 	}
+
 	/*
 	 * If we fall through, either no volume descriptor was found,
 	 * or else we passed a primary descriptor looking for others.
@@ -605,9 +604,7 @@
 	pri_bh = NULL;
 
 root_found:
-	brelse(pri_bh);
-
-	if (joliet_level && opt.rock == 'n') {
+	if (joliet_level && (pri == NULL || opt.rock == 'n')) {
 	    /* This is the case of Joliet with the norock mount flag.
 	     * A disc with both Joliet and Rock Ridge is handled later
 	     */
@@ -704,6 +701,7 @@
 	 * We're all done using the volume descriptor, and may need
 	 * to change the device blocksize, so release the buffer now.
 	 */
+	brelse(pri_bh);
 	brelse(bh);
 
 	/*
@@ -873,8 +871,8 @@
 /* Life is simpler than for other filesystem since we never
  * have to create a new block, only find an existing one.
  */
-int isofs_get_block(struct inode *inode, long iblock,
-		    struct buffer_head *bh_result, int create)
+static int isofs_get_block(struct inode *inode, long iblock,
+			   struct buffer_head *bh_result, int create)
 {
 	unsigned long b_off;
 	unsigned offset, sect_size;
diff -u --recursive --new-file ../linux-2.4.0-test11/linux/include/linux/iso_fs.h ./linux/include/linux/iso_fs.h
--- ../linux-2.4.0-test11/linux/include/linux/iso_fs.h	Tue Nov 21 21:44:16 2000
+++ ./linux/include/linux/iso_fs.h	Tue Nov 21 13:39:44 2000
@@ -185,7 +185,6 @@
 int get_acorn_filename(struct iso_directory_record *, char *, struct inode *);
 
 extern struct dentry *isofs_lookup(struct inode *, struct dentry *);
-extern int isofs_get_block(struct inode *, long, struct buffer_head *, int);
 extern int isofs_bmap(struct inode *, int);
 extern struct buffer_head *isofs_bread(struct inode *, unsigned int, unsigned int);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

silly [< >] and other excess

[Christian Gennerat]

Andries.Brouwer@cwi.nl a écrit :

>  I also left something else
> that always annoyed me: valuable screen space (on a 24x80 vt)
> is lost by these silly [< >] around addresses in an Oops.
> They provide no information at all, but on the other hand
> cause loss of information because these lines no longer
> fit in 80 columns causing line wrap and the loss of the
> top of the Oops.]
>

What a good idea!
Moreover, there is another problem in Oops:
the dumped stack is limited to 3 or 4 lines to prevent loss of information
but the call trace is unlimited and can loose all information,
and sometimes is printing forever!
--- arch/i386/kernel/traps.c.orig Mon Oct  2 20:57:01 2000
+++ arch/i386/kernel/traps.c Sun Nov  5 14:33:52 2000
@@ -142,11 +142,12 @@
    * out the call path that was taken.
    */
   if (((addr >= (unsigned long) &_stext) &&
+    (i<32) &&
        (addr <= (unsigned long) &_etext)) ||
       ((addr >= module_start) && (addr <= module_end))) {
    if (i && ((i % 8) == 0))
     printk("\n       ");
-   printk("[<%08lx>] ", addr);
+   printk("%08lx ", addr);
    i++;
   }
  }

And do not scroll the screen after the last printed line!

--- kernel/panic.c.orig Tue Jun 20 23:32:27 2000
+++ kernel/panic.c Sun Nov  5 07:53:04 2000
@@ -56,7 +56,7 @@
  va_end(args);
  printk(KERN_EMERG "Kernel panic: %s\n",buf);
  if (in_interrupt())
-  printk(KERN_EMERG "In interrupt handler - not syncing\n");
+  printk(KERN_EMERG "In interrupt handler - not syncing");
  else if (!current->pid)
   printk(KERN_EMERG "In idle task - not syncing\n");
  else


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: silly [< >] and other excess

[Russell King]

Christian Gennerat writes:
> Andries.Brouwer@cwi.nl a =E9crit :
> >  I also left something else
> > that always annoyed me: valuable screen space (on a 24x80 vt)
> > is lost by these silly [< >] around addresses in an Oops.
> > They provide no information at all, but on the other hand
> > cause loss of information because these lines no longer
> > fit in 80 columns causing line wrap and the loss of the
> > top of the Oops.]

They provide no information to the human reader, but they tell klogd
(and other tools) that the enclosed value is a kernel address that
should be looked up in the System.map file and decoded into name +
offset.

> What a good idea!

What a bad idea to remove them.

> --- arch/i386/kernel/traps.c.orig Mon Oct  2 20:57:01 2000
> +++ arch/i386/kernel/traps.c Sun Nov  5 14:33:52 2000
> @@ -142,11 +142,12 @@
>     * out the call path that was taken.
>     */
>    if (((addr >=3D (unsigned long) &_stext) &&
> +    (i<32) &&
>         (addr <=3D (unsigned long) &_etext)) ||
>        ((addr >=3D module_start) && (addr <=3D module_end))) {
>     if (i && ((i % 8) =3D=3D 0))
>      printk("\n       ");
> -   printk("[<%08lx>] ", addr);
> +   printk("%08lx ", addr);
>     i++;
>    }
>   }

What happened to the tabs?  It looks like you're using the deadly evil
quoted-printable mime encoding format which seems to h ave spannered the
patch.

> --- kernel/panic.c.orig Tue Jun 20 23:32:27 2000
> +++ kernel/panic.c Sun Nov  5 07:53:04 2000
> @@ -56,7 +56,7 @@
>   va_end(args);
>   printk(KERN_EMERG "Kernel panic: %s\n",buf);
>   if (in_interrupt())
> -  printk(KERN_EMERG "In interrupt handler - not syncing\n");
> +  printk(KERN_EMERG "In interrupt handler - not syncing");
>   else if (!current->pid)
>    printk(KERN_EMERG "In idle task - not syncing\n");
>   else

IMHO, panic here is a better idea; I've had many a time when I get oops
after oops.  Take for instance "scheduling in interrupt".  This causes
a NULL pointer de-reference, which then calls die() which goes on to call
do_exit() which then calls schedule() which then causes a NULL pointer
de-reference, which then calls die() which goes on to call do_exit() which
then calls schedule() which then causes a NULL pointer de-reference etc.

End result is a constant stream of oopsen fast scrolling by on screen until
such time something gets corrupted which breaks the loop.

PS, if you want to catch an oops which locks the machine, use a serial
console to log it.  If its a non-locking oops, examine the message log.
   _____
  |_____| ------------------------------------------------- ---+---+-
  |   |         Russell King        rmk@arm.linux.org.uk      --- ---
  | | | | http://www.arm.linux.org.uk/personal/aboutme.html   /  /  |
  | +-+-+                                                     --- -+-
  /   |               THE developer of ARM Linux              |+| /|\
 /  | | |                                                     ---  |
    +-+-+ -------------------------------------------------  /\\\  |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: silly [< >] and other excess

[Keith Owens]

On Wed, 22 Nov 2000 14:42:05 +0100, 
Christian Gennerat <christian.gennerat@vz.cit.alcatel.fr> wrote:
>Andries.Brouwer@cwi.nl a écrit :
>
>>  I also left something else
>> that always annoyed me: valuable screen space (on a 24x80 vt)
>> is lost by these silly [< >] around addresses in an Oops.
>> They provide no information at all, but on the other hand
>> cause loss of information because these lines no longer
>> fit in 80 columns causing line wrap and the loss of the
>> top of the Oops.]

You just broke ksymoops.  Removing the [< >] is a bad idea, they are
one of the few things that identifies the addresses in the log,
otherwise they just look like hex numbers.  ksymoops has to scan log
files which can contain anything and somehow pick out the interesting
lines, you need some identifier on the lines.

>Moreover, there is another problem in Oops:
>the dumped stack is limited to 3 or 4 lines to prevent loss of information
>but the call trace is unlimited and can loose all information,
>and sometimes is printing forever!
>--- arch/i386/kernel/traps.c.orig Mon Oct  2 20:57:01 2000
>+++ arch/i386/kernel/traps.c Sun Nov  5 14:33:52 2000
>@@ -142,11 +142,12 @@
>    * out the call path that was taken.
>    */
>   if (((addr >= (unsigned long) &_stext) &&
>+    (i<32) &&
>        (addr <= (unsigned long) &_etext)) ||
>       ((addr >= module_start) && (addr <= module_end))) {
>    if (i && ((i % 8) == 0))
>     printk("\n       ");
>-   printk("[<%08lx>] ", addr);
>+   printk("%08lx ", addr);
>    i++;
>   }
>  }

There should be no need to restrict the number of lines printed, it is
limited by the top of the kernel stack.  If there are more than 32
trace entries on the stack then they should be printed.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: silly [< >] and other excess

[Albert D. Cahalan]

Keith Owens writes:
> Christian Gennerat <christian.gennerat@vz.cit.alcatel.fr> wrote:
>> Andries.Brouwer@cwi.nl a =E9crit :

>>>  I also left something else
>>> that always annoyed me: valuable screen space (on a 24x80 vt)
>>> is lost by these silly [< >] around addresses in an Oops.
>>> They provide no information at all, but on the other hand
>>> cause loss of information because these lines no longer
>>> fit in 80 columns causing line wrap and the loss of the
>>> top of the Oops.]

> You just broke ksymoops.

You can fix it. Keeping useful info on the screen is more important.

> Removing the [< >] is a bad idea, they are
> one of the few things that identifies the addresses in the log,
> otherwise they just look like hex numbers.  ksymoops has to scan log
> files which can contain anything and somehow pick out the interesting
> lines, you need some identifier on the lines.

If you see register names followed by hex numbers, you have
some debug data. Scan forward and backward 25 lines, grabbing
all 8-digit and 16-digit hex numbers. Sort the numbers, then
look up all of them.

Crude solutions don't break as often as fancy solutions.

> There should be no need to restrict the number of lines printed, it is
> limited by the top of the kernel stack.  If there are more than 32
> trace entries on the stack then they should be printed.

It could fill the screen. There is an expansion of 4-to-13 when
using the silly brackets, and a PC stack can be 6 or 7 kB long,
or perhaps many megabytes due to stack overflow. The standard
VGA screen only allows 4000 bytes of data.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/