RE: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

RE: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Richard B. Johnson]

On Fri, 2 May 2003, Riley Williams wrote:

> Hi Richard.
>
>  >>>>> IOW, %d _does_ mean base=10.  base=0 is %i.  That goes
>  >>>>> both for kernel and userland implementations of scanf
>  >>>>> family (and for any standard-compliant implementation,
>  >>>>> for that matter).
>
>  > What!??????
>  >
>  > The base, in the kernel code is the number that you divide by
>  > to return the remainder for numerical conversions!  the base
>  > is 8 or octal, 10 for decimal, 16 for hexadecimal, and up to
>  > 36 for some other strange unused thing (all 26 letters of the
>  > alphabet).
>
> If you read the definition of the scanf() family of functions, you
> will note that a base of 0 is used to indicate that the standard C
> prefixes should be honoured, such that a number beginning with 0x
> is read as hexadecimal, a number beginning with 0 is read as octal
> and any other number is read as decimal. As a result, the value of
> 0 for base IS valid but is special cased.
>
> Think of it like the code that reads from a file. When the end of
> the file is reached, the "character" EOF (code -1) is returned.
> Such is not a valid character in the normal sense of the word, but
> a special case that the function can produce.
>
>  > If your conversion chances the base to 0, you divide by 0 (not
>  > good) and don't get a remainder. Actually  procedure number()
>  > protects against a base less than 2 or greater than 36 so you
>  > just prevent conversion altogether.
>
> If your conversion puts the base as 0, the code says "ah, we don't
> know what base to use, so we look at the first few digits to decide"
> and follows the above rules. Not a problem.
>
> Best wishes from Riley.
> ---

The base, shown in the definition in scanf() does not have any
correlation to the names of the internal variables used. The
current code, correctly uses the base of 10 when %d, %u, %l, and
all the other decimal variants are encountered, and correctly
uses base octal when %o is encountered, and correctly uses
base 16 when %x or its variations are encountered. The only
known problem is not a problem with base, but a problem with
scanning  the string because the standard allows 0x to preceed
a hexadecimal number.

Kevin Corry <kevcorry@us.ibm.com>, submitted a patch which breaks
already working code in an aparent attempt to fix the leading
"0x" problem.


Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Kevin Corry]

On Friday 02 May 2003 12:15, Richard B. Johnson wrote:
> Kevin Corry <kevcorry@us.ibm.com>, submitted a patch which breaks
> already working code in an aparent attempt to fix the leading
> "0x" problem.

It is not a patch to fix the leading "0x" problem. It is a patch to fix 
scanning of hex numbers which do *not* start with "0x". Hex numbers starting 
with "0x" will already scan correctly. Like I said before, try scanning "fe" 
as a hex number, and watch scanf return an error. Like I also said before, 
this same patch has been in 2.5 for quite a while, so I fail to see why it 
shouldn't also be applied to 2.4, unless we are specifically trying to 
maintain different behavior for scanf between the two kernel versions.

-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Richard B. Johnson]

On Fri, 2 May 2003, Jonathan Lundell wrote:

> At 11:27am -0400 5/2/03, Richard B. Johnson wrote:
> >If your conversion chances the base to 0, you divide by 0 (not
> >good) and don't get a remainder. Actually  procedure number()
> >protects against a base less than 2 or greater than 36 so you
> >just prevent conversion altogether.
>
> A bug in number(), looks like. (base = 0) is intended to let the
> input string determine the base (16 if 0x*, else 8 if 0* else 10).
> Like simple_strtol().
> --
No. number() is an internal helper function. The base is correctly
set before number() is called. The reported bug had nothing at
all to do with the internal variable, base. Instead it had to
do with not allowing a leading "0x" for hexadecimal numbers
as the standard allows (or has been interpreted to allow).

This is simply fixed by inspection of the string variable before
a conversion is attempted.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Jonathan Lundell]

At 11:27am -0400 5/2/03, Richard B. Johnson wrote:
>If your conversion chances the base to 0, you divide by 0 (not
>good) and don't get a remainder. Actually  procedure number()
>protects against a base less than 2 or greater than 36 so you
>just prevent conversion altogether.

A bug in number(), looks like. (base = 0) is intended to let the 
input string determine the base (16 if 0x*, else 8 if 0* else 10). 
Like simple_strtol().
-- 
/Jonathan Lundell.

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Richard B. Johnson]

On Fri, 2 May 2003, Kevin Corry wrote:

> On Friday 02 May 2003 04:50, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > On Fri, May 02, 2003 at 11:42:36AM +0200, Bodo Rzany wrote:
> > > > IOW, %d _does_ mean base=10.  base=0 is %i.  That goes both for kernel
> > > > and userland implementations of scanf family (and for any
> > > > standard-compliant implementation, for that matter).

What!??????

The base, in the kernel code is the number that you divide by
to return the remainder for numerical conversions!  the base
is 8 or octal, 10 for decimal, 16 for hexadecimal, and up to
36 for some other strange unused thing (all 26 letters of the
alphabet).

If your conversion chances the base to 0, you divide by 0 (not
good) and don't get a remainder. Actually  procedure number()
protects against a base less than 2 or greater than 36 so you
just prevent conversion altogether.


> > >
> > > As I can see, 'base=10' is used for all conversions except for '%x' and
> > > '%o'. If '%i' or '%u' are given, base should be really set to 0, what is
> > > not the case (it is fixed to 10 instead!).
> >

No No No!


Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Kevin Corry]

On Friday 02 May 2003 04:50, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Fri, May 02, 2003 at 11:42:36AM +0200, Bodo Rzany wrote:
> > > IOW, %d _does_ mean base=10.  base=0 is %i.  That goes both for kernel
> > > and userland implementations of scanf family (and for any
> > > standard-compliant implementation, for that matter).
> >
> > As I can see, 'base=10' is used for all conversions except for '%x' and
> > '%o'. If '%i' or '%u' are given, base should be really set to 0, what is
> > not the case (it is fixed to 10 instead!).
>
> Sorry - in 2.4 it's really broken.  What we should have:
> %d	->	10
> %i	->	0
> %o	->	8
> %u	->	10
> %x	->	16
> (note: %u is decimal-only; see manpage).
>
> Fix (2.4-only, 2.5 is OK as it is):
>
> --- S21-rc1/lib/vsprintf.c	Fri Jul 12 11:25:33 2002
> +++ /tmp/vsprintf.c	Fri May  2 05:46:07 2003
> @@ -616,8 +616,9 @@
>  		case 'X':
>  			base = 16;
>  			break;
> -		case 'd':
>  		case 'i':
> +			base = 0;
> +		case 'd':
>  			is_sign = 1;
>  		case 'u':
>  			break;

In order to get the hex and octal scanning correct, the following patch
also needs to be applied. For example, trying to scan "fe" with "%x"
currently fails. This is the same change that was made in 2.5.

diff -Naur linux-2.4.20a/lib/vsprintf.c linux-2.4.20b/lib/vsprintf.c
--- linux-2.4.20a/lib/vsprintf.c	2003-04-28 12:04:44.000000000 -0500
+++ linux-2.4.20b/lib/vsprintf.c	2003-04-28 12:04:25.000000000 -0500
@@ -637,7 +637,11 @@
 		while (isspace(*str))
 			str++;
 
-		if (!*str || !isdigit(*str))
+		if (!*str
+		    || (base == 16 && !isxdigit(*str))
+		    || (base == 10 && !isdigit(*str))
+		    || (base == 8 && (!isdigit(*str) || *str > '7'))
+		    || (base == 0 && !isdigit(*str)))
 			break;
 
 		switch(qualifier) {


-- 
Kevin Corry
kevcorry@us.ibm.com
http://evms.sourceforge.net/

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[viro]

On Fri, May 02, 2003 at 11:42:36AM +0200, Bodo Rzany wrote:

> > IOW, %d _does_ mean base=10.  base=0 is %i.  That goes both for kernel and
> > userland implementations of scanf family (and for any standard-compliant
> > implementation, for that matter).
> 
> As I can see, 'base=10' is used for all conversions except for '%x' and
> '%o'. If '%i' or '%u' are given, base should be really set to 0, what is
> not the case (it is fixed to 10 instead!).

Sorry - in 2.4 it's really broken.  What we should have:
%d	->	10
%i	->	0
%o	->	8
%u	->	10
%x	->	16
(note: %u is decimal-only; see manpage).

Fix (2.4-only, 2.5 is OK as it is):

--- S21-rc1/lib/vsprintf.c	Fri Jul 12 11:25:33 2002
+++ /tmp/vsprintf.c	Fri May  2 05:46:07 2003
@@ -616,8 +616,9 @@
 		case 'X':
 			base = 16;
 			break;
-		case 'd':
 		case 'i':
+			base = 0;
+		case 'd':
 			is_sign = 1;
 		case 'u':
 			break;

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Bodo Rzany]

On Fri, 2 May 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:

> On Fri, May 02, 2003 at 11:06:32AM +0200, Bodo Rzany wrote:
> > PROBLEM:
> > 	Hex/Octal decoding with sscanf from kernel library does not work
> > 	within kernel 2.4.20
> >
> > DESCRIPTION:
> > 	Line 570 in lib/vsprintf.c
> >
> > 			14677 11. Okt 2001  vsprintf.c
> >
> > 	holds '	base = 10; '
> >
> > 	which prevents the real conversion routines ('simple_strtoul' a.s.o.)
> > 	from decoding numbers from bases other than 10.
>
> It's not a problem, it's standard-mandated behaviour.

I don't think so. Please read a few lines below:

>
> >From vsscanf(3):
>        The following conversions are available:
> ...
>        d      Matches  an  optionally signed decimal integer; the next pointer
>               must be a pointer to int.
> ...
>        i      Matches an optionally signed integer; the next pointer must be a
>               pointer  to  int.   The  integer is read in base 16 if it begins
>               with `0x' or `0X', in base 8 if it begins with `0', and in  base
>               10  otherwise.   Only characters that correspond to the base are
>               used.

If one tries to read an '0xabc' or even '0732' entry in the data buffer,
sscanf returns '-1'. The problem seems to be that 'sscanf' claims for
'base=10' every time, and this prevents 'simple_strtoul' (and the other
conversion routines) from extracting the desired base out of the input
string (format string declarations works fine, as you stated!).

> IOW, %d _does_ mean base=10.  base=0 is %i.  That goes both for kernel and
> userland implementations of scanf family (and for any standard-compliant
> implementation, for that matter).

As I can see, 'base=10' is used for all conversions except for '%x' and
'%o'. If '%i' or '%u' are given, base should be really set to 0, what is
not the case (it is fixed to 10 instead!).

Re: is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[viro]

On Fri, May 02, 2003 at 11:06:32AM +0200, Bodo Rzany wrote:
> PROBLEM:
> 	Hex/Octal decoding with sscanf from kernel library does not work
> 	within kernel 2.4.20
> 
> DESCRIPTION:
> 	Line 570 in lib/vsprintf.c
> 
> 			14677 11. Okt 2001  vsprintf.c
> 
> 	holds '	base = 10; '
> 
> 	which prevents the real conversion routines ('simple_strtoul' a.s.o.)
> 	from decoding numbers from bases other than 10.

It's not a problem, it's standard-mandated behaviour.

>From vsscanf(3):
       The following conversions are available:
...
       d      Matches  an  optionally signed decimal integer; the next pointer
              must be a pointer to int.
...
       i      Matches an optionally signed integer; the next pointer must be a
              pointer  to  int.   The  integer is read in base 16 if it begins
              with `0x' or `0X', in base 8 if it begins with `0', and in  base
              10  otherwise.   Only characters that correspond to the base are
              used.

IOW, %d _does_ mean base=10.  base=0 is %i.  That goes both for kernel and
userland implementations of scanf family (and for any standard-compliant
implementation, for that matter).

is there small mistake in lib/vsprintf.c of kernel 2.4.20 ?

[Bodo Rzany]

PROBLEM:
	Hex/Octal decoding with sscanf from kernel library does not work
	within kernel 2.4.20

DESCRIPTION:
	Line 570 in lib/vsprintf.c

			14677 11. Okt 2001  vsprintf.c

	holds '	base = 10; '

	which prevents the real conversion routines ('simple_strtoul' a.s.o.)
	from decoding numbers from bases other than 10.

(possible?) SOLUTION:

	Changing line 570 from ' base = 10; ' to ' base = 0;' works for me.
	I have found no side effects up to now, but I am not sure that there
	might be some.

I am by no means a kernel professional or an experienced c-programmer, so I
hope that some of the linux gurus can have a look at this little problem.

I am sorry not to have any sample code for you, that shows the error.
I have discovered the problem on writing a small kernel module for driving
a home brewed TUSB3410-development-board, and this modul needs the special
hardware to be functional.

If there should be some interest on this kernel module for USB hard- and
software development (the TUSB3410 from Texas Instruments is a programmable
USB to serial converter), I would be proud to release this small work to the
public (including the hardware and, if of further interest, the TUSB-code).

Thanks for all the good linux code I can work with, and

Best regards

Bodo Rzany
<bodo@rzany.de>