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

[Willy Tarreau <willy@w.ods.org>]

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;
>  		}