[PATCH] 2.4.31: fix isofs mount options parser

[PATCH] 2.4.31: fix isofs mount options parser

[Andrey J. Melnikoff (TEMHOTA)]

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]


Hello Marcelo, LKML.

Attached patch fix this whatings from gcc-3.4 and allow user mount
isofs with "session" and "sbsector" options. Without this patch, gcc-3.4
optimizer always return zero.

--- cut ---
inode.c: In function `parse_options':
inode.c:341: warning: comparison of unsigned expression < 0 is always false
inode.c:347: warning: comparison of unsigned expression < 0 is always false
--- cut ---

Signed-of-by: Andrey Melnikoff <temnota@kmv.ru>

--- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
+++ linux-2.4.31/fs/isofs/inode.c	2005-08-10 16:19:11.000000000 +0400
@@ -337,13 +337,13 @@
 		}
 		if (!strcmp(this_char,"session") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >99) return 0;
 			popt->session=ivalue+1;
 		}
 		if (!strcmp(this_char,"sbsector") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >660*512) return 0;
 			popt->sbsector=ivalue;
 		}

-- 
 Best regards, TEMHOTA-RIPN aka MJA13-RIPE
 System Administrator. mailto:temnota@kmv.ru


[-- Attachment #2: isofs-fix.diff --]
[-- Type: text/plain, Size: 688 bytes --]

--- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
+++ linux-2.4.31/fs/isofs/inode.c	2005-08-10 16:19:11.000000000 +0400
@@ -337,13 +337,13 @@
 		}
 		if (!strcmp(this_char,"session") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >99) return 0;
 			popt->session=ivalue+1;
 		}
 		if (!strcmp(this_char,"sbsector") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >660*512) return 0;
 			popt->sbsector=ivalue;
 		}

Re: [PATCH] 2.4.31: fix isofs mount options parser

[Willy Tarreau]

Hello Andrey,

On Wed, Aug 10, 2005 at 04:46:54PM +0400, Andrey J. Melnikoff (TEMHOTA) wrote:
> 
> Hello Marcelo, LKML.
> 
> Attached patch fix this whatings from gcc-3.4 and allow user mount
> isofs with "session" and "sbsector" options. Without this patch, gcc-3.4
> optimizer always return zero.

It should not, or it's a bug, because the first test (ivalue < 0) is false,
so the second one (ivalue > 99) must be evaluated before returning anything.

You patch is not the correct way to fix it either, because simple_strtoul()
is defined as unsigned long. So with your patch, very large values of ivalue
will be converted to negative and checked as invalid. In fact, you should have
changed the ivalue type to unsigned long, and removed the (ivalue < 0) test.

Please repost it fixed, I should have time to merge it into the next hotfix.

Thanks,
Willy


> 
> --- cut ---
> inode.c: In function `parse_options':
> inode.c:341: warning: comparison of unsigned expression < 0 is always false
> inode.c:347: warning: comparison of unsigned expression < 0 is always false
> --- cut ---
> 
> Signed-of-by: Andrey Melnikoff <temnota@kmv.ru>
> 
> --- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
> +++ linux-2.4.31/fs/isofs/inode.c	2005-08-10 16:19:11.000000000 +0400
> @@ -337,13 +337,13 @@
>  		}
>  		if (!strcmp(this_char,"session") && value) {
>  			char * vpnt = value;
> -			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
> +			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>  			if(ivalue < 0 || ivalue >99) return 0;
>  			popt->session=ivalue+1;
>  		}
>  		if (!strcmp(this_char,"sbsector") && value) {
>  			char * vpnt = value;
> -			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
> +			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>  			if(ivalue < 0 || ivalue >660*512) return 0;
>  			popt->sbsector=ivalue;
>  		}
> 
> -- 
>  Best regards, TEMHOTA-RIPN aka MJA13-RIPE
>  System Administrator. mailto:temnota@kmv.ru
> 

> --- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
> +++ linux-2.4.31/fs/isofs/inode.c	2005-08-10 16:19:11.000000000 +0400
> @@ -337,13 +337,13 @@
>  		}
>  		if (!strcmp(this_char,"session") && value) {
>  			char * vpnt = value;
> -			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
> +			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>  			if(ivalue < 0 || ivalue >99) return 0;
>  			popt->session=ivalue+1;
>  		}
>  		if (!strcmp(this_char,"sbsector") && value) {
>  			char * vpnt = value;
> -			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
> +			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>  			if(ivalue < 0 || ivalue >660*512) return 0;
>  			popt->sbsector=ivalue;
>  		}

[PATCH] 2.4.31: fix isofs mount options parser

[Andrey J. Melnikoff (TEMHOTA)]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]

Hello Marcelo, LKML.

Previos patch incomplete and fix only gcc warnings. New patch attached.

Description:
This patch pix gcc-3.4 warnings, andfix broken logic in mount options
parser.

--- cut ---
inode.c: In function `parse_options':
inode.c:341: warning: comparison of unsigned expression < 0 is always false
inode.c:347: warning: comparison of unsigned expression < 0 is always false
--- cut ---

Signed-of-by: Andrey Melnikoff <temnota@kmv.ru>

--- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
+++ linux-2.4.31/fs/isofs/inode.c	2005-08-11 13:55:25.000000000 +0400
@@ -335,15 +335,15 @@
 			else if (!strcmp(value,"acorn")) popt->map = 'a';
 			else return 0;
 		}
-		if (!strcmp(this_char,"session") && value) {
+		else if (!strcmp(this_char,"session") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >99) return 0;
 			popt->session=ivalue+1;
 		}
-		if (!strcmp(this_char,"sbsector") && value) {
+		else if (!strcmp(this_char,"sbsector") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >660*512) return 0;
 			popt->sbsector=ivalue;
 		}

-- 
 Best regards, TEMHOTA-RIPN aka MJA13-RIPE
 System Administrator. mailto:temnota@kmv.ru


[-- Attachment #2: isofs-fix.diff --]
[-- Type: text/plain, Size: 868 bytes --]

--- linux-2.4.31/fs/isofs/inode.c~old	2005-08-10 16:18:48.000000000 +0400
+++ linux-2.4.31/fs/isofs/inode.c	2005-08-11 13:55:25.000000000 +0400
@@ -335,15 +335,15 @@
 			else if (!strcmp(value,"acorn")) popt->map = 'a';
 			else return 0;
 		}
-		if (!strcmp(this_char,"session") && value) {
+		else if (!strcmp(this_char,"session") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >99) return 0;
 			popt->session=ivalue+1;
 		}
-		if (!strcmp(this_char,"sbsector") && value) {
+		else if (!strcmp(this_char,"sbsector") && value) {
 			char * vpnt = value;
-			unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
+			int ivalue = simple_strtoul(vpnt, &vpnt, 0);
 			if(ivalue < 0 || ivalue >660*512) return 0;
 			popt->sbsector=ivalue;
 		}