Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Ray Lee]

> > What should it do?
> I would model this after user space. (Which does strange things: 

<snip>

It only sounds strange at first. It actually means that scanf is
consistent with C's rules of assignment between mixed types. For
example:

ray:~$ cat signs.c

#include <stdio.h>

main() {
	char scan[]="-100";
	unsigned int u;
	int i;

	sscanf(scan, "%ud", &u);
	sscanf(scan, "%d", &i);
	printf("%s scanned to signed %d and unsigned %u\n", scan, i, u);

	i=-100;
	u=i;
	printf("%d assigned to unsigned int gives %u\n", i, u);
}

ray:~$ ./signs
-100 scanned to signed -100 and unsigned 4294967196
-100 assigned to unsigned int gives 4294967196

So, one should think of scanf as having correct knowledge of the types
it's scanning, and then shoe-horning the result into whatever you asked
for. Just like C itself.

Ray

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On 11 Nov 2002, Ray Lee wrote:

Randy wrote:
| > > What should it do?
Henning wrote:
| > I would model this after user space. (Which does strange things:
|
| <snip>
|
| It only sounds strange at first. It actually means that scanf is
| consistent with C's rules of assignment between mixed types. For
| example:
|
| ray:~$ cat signs.c
|
[snippage]
|
| ray:~$ ./signs
| -100 scanned to signed -100 and unsigned 4294967196
| -100 assigned to unsigned int gives 4294967196

These values (-100 and 4294967196) are the same bits, just observed/used
in a different manner.

| So, one should think of scanf as having correct knowledge of the types
| it's scanning, and then shoe-horning the result into whatever you asked
| for. Just like C itself.

Why would scanf() have to know whether the destination is signed
or unsigned?  It needs to know type (=> size) of course.

Seems to me that "signed-ness" is in the eyes of the user of the value.
I.e., scanf() can scan "-100" to an <int value> of
-100 = (unsigned) 4294967196 and shoe-horn that value into an
int-sized shoe and the caller can use it as signed or unsigned.


Andreas Schwab wrote:
|> IOW, asking for an unsigned number (in the format string)
|> and getting "-123" does return 0.

| Not in C.  According to the standard scanf is supposed to convert the
| value to unsinged and return that.
OK, thanks, I found that in the C spec.

Now what does it mean to "convert the value to an unsigned and return
that."  This is the same as above, isn't it?
I.e., on the scanf() side, there is no conversion needed; just store the
value.

So... somebody tell me if the following patch makes the kernel's
vsscanf() function act like C's sscanf() function, or why it doesn't,
or what I'm overlooking.  :)

And then we'll see if the change is worthwhile... or for which kernel
version.
-- 
~Randy




--- ./lib/vsprintf.c%scan	Sun Nov 10 19:28:30 2002
+++ ./lib/vsprintf.c	Mon Nov 11 09:56:51 2002
@@ -640,7 +640,7 @@
 			str++;

 		digit = *str;
-		if (is_sign && digit == '-')
+		if (digit == '-')
 			digit = *(str + 1);

 		if (!digit
@@ -652,45 +652,33 @@

 		switch(qualifier) {
 		case 'h':
-			if (is_sign) {
-				short *s = (short *) va_arg(args,short *);
-				*s = (short) simple_strtol(str,&next,base);
-			} else {
-				unsigned short *s = (unsigned short *) va_arg(args, unsigned short *);
-				*s = (unsigned short) simple_strtoul(str, &next, base);
+			{
+			short *s = (short *) va_arg(args,short *);
+			*s = (short) simple_strtol(str,&next,base);
 			}
 			break;
 		case 'l':
-			if (is_sign) {
-				long *l = (long *) va_arg(args,long *);
-				*l = simple_strtol(str,&next,base);
-			} else {
-				unsigned long *l = (unsigned long*) va_arg(args,unsigned long*);
-				*l = simple_strtoul(str,&next,base);
+			{
+			long *l = (long *) va_arg(args,long *);
+			*l = simple_strtol(str,&next,base);
 			}
 			break;
 		case 'L':
-			if (is_sign) {
-				long long *l = (long long*) va_arg(args,long long *);
-				*l = simple_strtoll(str,&next,base);
-			} else {
-				unsigned long long *l = (unsigned long long*) va_arg(args,unsigned long long*);
-				*l = simple_strtoull(str,&next,base);
+			{
+			long long *l = (long long*) va_arg(args,long long *);
+			*l = simple_strtoll(str,&next,base);
 			}
 			break;
 		case 'Z':
-		{
+			{
 			size_t *s = (size_t*) va_arg(args,size_t*);
 			*s = (size_t) simple_strtoul(str,&next,base);
-		}
-		break;
+			}
+			break;
 		default:
-			if (is_sign) {
-				int *i = (int *) va_arg(args, int*);
-				*i = (int) simple_strtol(str,&next,base);
-			} else {
-				unsigned int *i = (unsigned int*) va_arg(args, unsigned int*);
-				*i = (unsigned int) simple_strtoul(str,&next,base);
+			{
+			int *i = (int *) va_arg(args, int*);
+			*i = (int) simple_strtol(str,&next,base);
 			}
 			break;
 		}

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Andreas Schwab]

"Randy.Dunlap" <rddunlap@osdl.org> writes:

|> Andreas Schwab wrote:
|> |> IOW, asking for an unsigned number (in the format string)
|> |> and getting "-123" does return 0.
|> 
|> | Not in C.  According to the standard scanf is supposed to convert the
|> | value to unsinged and return that.
|> OK, thanks, I found that in the C spec.
|> 
|> Now what does it mean to "convert the value to an unsigned and return
|> that."  This is the same as above, isn't it?
|> I.e., on the scanf() side, there is no conversion needed; just store the
|> value.

The C standard also supports ones-complement and sign-magnitude
representation of signed integers where signed<->unsigned conversion is a
non-trivial operation in the sense that the bit representation does
change.  And scanf knows the signedness of the destination due to the
format spec.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Ray Lee]

On Mon, 2002-11-11 at 11:09, Randy.Dunlap wrote:
> | It only sounds strange at first. It actually means that scanf is
> | consistent with C's rules of assignment between mixed types. For
> | example:
<snip>
> These values (-100 and 4294967196) are the same bits, just observed/used
> in a different manner.

I probably didn't state it very clearly, but that was what I was getting
at. It's the same value after a cast. As Andreas points out though, it
may not be bit-for-bit identical on machines that don't use two's
complement. That's why the conversion code cares about the format
specifiers, it seems.

> Now what does it mean to "convert the value to an unsigned and return
> that."  This is the same as above, isn't it?

Explicitly, in the scan conversion you'd do a:

  unsigned int *u = (unsigned int *) va_arg(args,long long *);
  *u = (unsigned int) converted_value;

...from wherever you get converted_value from (simple_strtoul? I haven't
looked at that routine). (The cast here is implied if left off, I'm just
being explicit.)

> I.e., on the scanf() side, there is no conversion needed; just store the
> value.

Almost true, as Andreas pointed out. You have to be careful that the
target you're storing to has the correctly declared type inside the
conversion routine. 

Ray

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On 11 Nov 2002, Ray Lee wrote:

| I probably didn't state it very clearly, but that was what I was getting
| at. It's the same value after a cast. As Andreas points out though, it
| may not be bit-for-bit identical on machines that don't use two's
| complement. That's why the conversion code cares about the format
| specifiers, it seems.
|
| > Now what does it mean to "convert the value to an unsigned and return
| > that."  This is the same as above, isn't it?
|
| Explicitly, in the scan conversion you'd do a:
|
|   unsigned int *u = (unsigned int *) va_arg(args,long long *);
|   *u = (unsigned int) converted_value;
|
| ...from wherever you get converted_value from (simple_strtoul? I haven't
| looked at that routine). (The cast here is implied if left off, I'm just
| being explicit.)
|
| > I.e., on the scanf() side, there is no conversion needed; just store the
| > value.
|
| Almost true, as Andreas pointed out. You have to be careful that the
| target you're storing to has the correctly declared type inside the
| conversion routine.

See if this is close...

For 'h' (short), 'l' (long), 'L' (long long), and default
('d' and 'i'), signed input is allowed, even for octal or hex
(as well as decimal), so scan/convert signed values.
Then if the destination type is not signed (!is_sign),
store the value in an unsigned <length> var so that
conversion can be done as needed.

Here are a few sample conversions:

scanning input string:{-42}:
  %o level = 037777777736 = 0xffffffde
  %x level = -66 = 0xffffffbe
  %i level = -42 = 0xffffffd6
scanning input string:{-100}:
  %o level = 037777777700 = 0xffffffc0
  %x level = -256 = 0xffffff00
  %i level = -100 = 0xffffff9c
scanning input string:{-10}:
  %o level = 037777777770 = 0xfffffff8
  %x level = -16 = 0xfffffff0
  %i level = -10 = 0xfffffff6

I think that this patch (to 2.5.47) gets the kernel close
to the same semantics as C's sscanf() function, which is
usually a good thing.  What say you?

-- 
~Randy
  "I read part of it all the way through." -- Samuel Goldwyn




--- ./lib/vsprintf.c%scan	Sun Nov 10 19:28:30 2002
+++ ./lib/vsprintf.c	Tue Nov 12 20:51:23 2002
@@ -640,7 +640,7 @@
 			str++;

 		digit = *str;
-		if (is_sign && digit == '-')
+		if (digit == '-')
 			digit = *(str + 1);

 		if (!digit
@@ -652,46 +652,50 @@

 		switch(qualifier) {
 		case 'h':
-			if (is_sign) {
-				short *s = (short *) va_arg(args,short *);
-				*s = (short) simple_strtol(str,&next,base);
-			} else {
-				unsigned short *s = (unsigned short *) va_arg(args, unsigned short *);
-				*s = (unsigned short) simple_strtoul(str, &next, base);
+		{
+			short *s = (short *) va_arg(args, short *);
+			*s = (short) simple_strtol(str, &next, base);
+			if (!is_sign) {
+				unsigned short *us = s;
+				*us = (unsigned short) *s;
 			}
+		}
 			break;
 		case 'l':
-			if (is_sign) {
-				long *l = (long *) va_arg(args,long *);
-				*l = simple_strtol(str,&next,base);
-			} else {
-				unsigned long *l = (unsigned long*) va_arg(args,unsigned long*);
-				*l = simple_strtoul(str,&next,base);
+		{
+			long *l = (long *) va_arg(args, long *);
+			*l = simple_strtol(str, &next, base);
+			if (!is_sign) {
+				unsigned long *ul = l;
+				*ul = (unsigned long) *l;
 			}
+		}
 			break;
 		case 'L':
-			if (is_sign) {
-				long long *l = (long long*) va_arg(args,long long *);
-				*l = simple_strtoll(str,&next,base);
-			} else {
-				unsigned long long *l = (unsigned long long*) va_arg(args,unsigned long long*);
-				*l = simple_strtoull(str,&next,base);
+		{
+			long long *l = (long long *) va_arg(args, long long *);
+			*l = simple_strtoll(str, &next, base);
+			if (!is_sign) {
+				unsigned long long *ul = l;
+				*ul = (unsigned long long) *l;
 			}
+		}
 			break;
 		case 'Z':
 		{
-			size_t *s = (size_t*) va_arg(args,size_t*);
-			*s = (size_t) simple_strtoul(str,&next,base);
+			size_t *s = (size_t *) va_arg(args, size_t *);
+			*s = (size_t) simple_strtoul(str, &next, base);
 		}
-		break;
+			break;
 		default:
-			if (is_sign) {
-				int *i = (int *) va_arg(args, int*);
-				*i = (int) simple_strtol(str,&next,base);
-			} else {
-				unsigned int *i = (unsigned int*) va_arg(args, unsigned int*);
-				*i = (unsigned int) simple_strtoul(str,&next,base);
+		{
+			int *i = (int *) va_arg(args, int *);
+			*i = (int) simple_strtol(str, &next, base);
+			if (!is_sign) {
+				unsigned int *ui = i;
+				*ui = (unsigned int) *i;
 			}
+		}
 			break;
 		}
 		num++;

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Ray Lee]

(Sorry for taking so long to review something so short, btw.)

On Tue, 2002-11-12 at 23:06, Randy.Dunlap wrote:
> On 11 Nov 2002, Ray Lee wrote:
> | Explicitly, in the scan conversion you'd do a:
> |   unsigned int *u = (unsigned int *) va_arg(args,long long *);
> |   *u = (unsigned int) converted_value;

(Luckily you didn't follow my code snippet too closely. Oops.)

> See if this is close...
<snip>
> I think that this patch (to 2.5.47) gets the kernel close
> to the same semantics as C's sscanf() function, which is
> usually a good thing.  What say you?

The sample conversions and patch look correct. Time to forward it
onward, me thinks.

Ray

sscanf("-1", "%d", &i) fails, returns 0

[Douglas Gilbert]

In lk 2.5.46-bk3 the expression in the subject line
fails to write into "i" and returns 0. Drop the minus
sign and it works.

Doug Gilbert

[PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On Sat, 9 Nov 2002, Douglas Gilbert wrote:

| In lk 2.5.46-bk3 the expression in the subject line
| fails to write into "i" and returns 0. Drop the minus
| sign and it works.

Here's an unobstrusive patch to correct that.
Please apply.

-- 
~Randy



--- ./lib/vsprintf.c%signed	Mon Nov  4 14:30:49 2002
+++ ./lib/vsprintf.c	Fri Nov  8 11:20:03 2002
@@ -517,6 +517,7 @@
 {
 	const char *str = buf;
 	char *next;
+	char *dig;
 	int num = 0;
 	int qualifier;
 	int base;
@@ -638,12 +639,13 @@
 		while (isspace(*str))
 			str++;

-		if (!*str
-                    || (base == 16 && !isxdigit(*str))
-                    || (base == 10 && !isdigit(*str))
-                    || (base == 8 && (!isdigit(*str) || *str > '7'))
-                    || (base == 0 && !isdigit(*str)))
-			break;
+		dig = (*str == '-') ? (str + 1) : str;
+		if (!*dig
+                    || (base == 16 && !isxdigit(*dig))
+                    || (base == 10 && !isdigit(*dig))
+                    || (base == 8 && (!isdigit(*dig) || *dig > '7'))
+                    || (base == 0 && !isdigit(*dig)))
+				break;

 		switch(qualifier) {
 		case 'h':

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Richard B. Johnson]

On Fri, 8 Nov 2002, Randy.Dunlap wrote:

> On Sat, 9 Nov 2002, Douglas Gilbert wrote:
> 
> | In lk 2.5.46-bk3 the expression in the subject line
> | fails to write into "i" and returns 0. Drop the minus
> | sign and it works.
> 
> Here's an unobstrusive patch to correct that.
> Please apply.
> 
> -- 
> ~Randy
> 
> 
> 
> --- ./lib/vsprintf.c%signed	Mon Nov  4 14:30:49 2002
> +++ ./lib/vsprintf.c	Fri Nov  8 11:20:03 2002
> @@ -517,6 +517,7 @@
>  {
>  	const char *str = buf;
>  	char *next;
> +	char *dig;
>  	int num = 0;
>  	int qualifier;
>  	int base;
> @@ -638,12 +639,13 @@
>  		while (isspace(*str))
>  			str++;
> 
> -		if (!*str
> -                    || (base == 16 && !isxdigit(*str))
> -                    || (base == 10 && !isdigit(*str))
> -                    || (base == 8 && (!isdigit(*str) || *str > '7'))
> -                    || (base == 0 && !isdigit(*str)))
> -			break;
> +		dig = (*str == '-') ? (str + 1) : str;
> +		if (!*dig
> +                    || (base == 16 && !isxdigit(*dig))
> +                    || (base == 10 && !isdigit(*dig))
> +                    || (base == 8 && (!isdigit(*dig) || *dig > '7'))
> +                    || (base == 0 && !isdigit(*dig)))
> +				break;
> 
>  		switch(qualifier) {
>  		case 'h':
> 
> -

I was thinking that if anybody ever had to change any of this
stuff, it might be a good idea to do the indirection only once?
All those "splats" over and over again are costly.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
   Bush : The Fourth Reich of America

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On Fri, 8 Nov 2002, Richard B. Johnson wrote:

| On Fri, 8 Nov 2002, Randy.Dunlap wrote:
|
| > On Sat, 9 Nov 2002, Douglas Gilbert wrote:
| >
| > | In lk 2.5.46-bk3 the expression in the subject line
| > | fails to write into "i" and returns 0. Drop the minus
| > | sign and it works.
| >
| > Here's an unobstrusive patch to correct that.
| > Please apply.
| >
| > --
| > ~Randy
| > -
|
| I was thinking that if anybody ever had to change any of this
| stuff, it might be a good idea to do the indirection only once?
| All those "splats" over and over again are costly.

Sure, it looks cleaner that way, although gcc has already put <*dig>
in a local register; i.e., it's not pulled from memory for each test.
Here's a (tested) version that does that.

-- 
~Randy



--- ./lib/vsprintf.c%signed	Mon Nov  4 14:30:49 2002
+++ ./lib/vsprintf.c	Fri Nov  8 12:03:15 2002
@@ -517,6 +517,7 @@
 {
 	const char *str = buf;
 	char *next;
+	char *dig, onedig;
 	int num = 0;
 	int qualifier;
 	int base;
@@ -638,12 +639,14 @@
 		while (isspace(*str))
 			str++;

-		if (!*str
-                    || (base == 16 && !isxdigit(*str))
-                    || (base == 10 && !isdigit(*str))
-                    || (base == 8 && (!isdigit(*str) || *str > '7'))
-                    || (base == 0 && !isdigit(*str)))
-			break;
+		dig = (*str == '-') ? (str + 1) : str;
+		onedig = *dig;
+		if (!onedig
+                    || (base == 16 && !isxdigit(onedig))
+                    || (base == 10 && !isdigit(onedig))
+                    || (base == 8 && (!isdigit(onedig) || onedig > '7'))
+                    || (base == 0 && !isdigit(onedig)))
+				break;

 		switch(qualifier) {
 		case 'h':

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Linus Torvalds]

On Fri, 8 Nov 2002, Randy.Dunlap wrote:
?
> Sure, it looks cleaner that way, although gcc has already put <*dig>
> in a local register; i.e., it's not pulled from memory for each test.
> Here's a (tested) version that does that.

Why do you have that "dig" pointer at all? It's not really used.

Why not just do

	+	char digit;
	...

	+	digit = str;
	+	if (digit == '-')
	+		digit = str[1];


(and maybe it should also test for whether signed stuff is even alloed or 
not, ie maybe the test should be "if (is_sign && digit == '-')" instead)

		Linus

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On Fri, 8 Nov 2002, Linus Torvalds wrote:

|
| On Fri, 8 Nov 2002, Randy.Dunlap wrote:
| ?
| > Sure, it looks cleaner that way, although gcc has already put <*dig>
| > in a local register; i.e., it's not pulled from memory for each test.
| > Here's a (tested) version that does that.
|
| Why do you have that "dig" pointer at all? It's not really used.
|
| Why not just do
|
| 	+	char digit;
| 	...
|
| 	+	digit = str;
| 	+	if (digit == '-')
| 	+		digit = str[1];
|
|
| (and maybe it should also test for whether signed stuff is even alloed or
| not, ie maybe the test should be "if (is_sign && digit == '-')" instead)

OK, I've cleaned it up as suggested...

-- 
~Randy



--- ./lib/vsprintf.c%signed	Mon Nov  4 14:30:49 2002
+++ ./lib/vsprintf.c	Fri Nov  8 13:54:33 2002
@@ -517,6 +517,7 @@
 {
 	const char *str = buf;
 	char *next;
+	char digit;
 	int num = 0;
 	int qualifier;
 	int base;
@@ -638,12 +639,16 @@
 		while (isspace(*str))
 			str++;

-		if (!*str
-                    || (base == 16 && !isxdigit(*str))
-                    || (base == 10 && !isdigit(*str))
-                    || (base == 8 && (!isdigit(*str) || *str > '7'))
-                    || (base == 0 && !isdigit(*str)))
-			break;
+		digit = *str;
+		if (is_sign && digit == '-')
+			digit = *(str + 1);
+
+		if (!digit
+                    || (base == 16 && !isxdigit(digit))
+                    || (base == 10 && !isdigit(digit))
+                    || (base == 8 && (!isdigit(digit) || digit > '7'))
+                    || (base == 0 && !isdigit(digit)))
+				break;

 		switch(qualifier) {
 		case 'h':

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Henning P. Schmiedehausen]

"Randy.Dunlap" <rddunlap@osdl.org> writes:

>+		digit = *str;
>+		if (is_sign && digit == '-')
>+			digit = *(str + 1);

If signed is not allowed and you get a "-", you're in an error case
again...

	Regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     hps@intermeta.de

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   info@intermeta.de
D-91054 Buckenhof     Fax.: 09131 / 50654-20   

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:

| "Randy.Dunlap" <rddunlap@osdl.org> writes:
|
| >+		digit = *str;
| >+		if (is_sign && digit == '-')
| >+			digit = *(str + 1);
|
| If signed is not allowed and you get a "-", you're in an error case
| again...

Yes, and a 0 value is returned.
IOW, asking for an unsigned number (in the format string)
and getting "-123" does return 0.

What should it do?
This function can't return -EINPUTERROR or -EILSEQ.
(since it's after feature-freeze :)
And the original problem was that a leading '-' sign on a
signed number (!) caused a return of 0.  At least that is fixed.

So now the problem (?) is that a '-' sign on an unsigned number
returns 0.  We can always add a big printk() there that
something is foul.  Other ideas?

-- 
~Randy

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Randy.Dunlap]

On Sun, 10 Nov 2002, Randy.Dunlap wrote:

| On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:
|
| | "Randy.Dunlap" <rddunlap@osdl.org> writes:
| |
| | >+		digit = *str;
| | >+		if (is_sign && digit == '-')
| | >+			digit = *(str + 1);
| |
| | If signed is not allowed and you get a "-", you're in an error case
| | again...
|
| Yes, and a 0 value is returned.
| IOW, asking for an unsigned number (in the format string)
| and getting "-123" does return 0.
|
| What should it do?
| This function can't return -EINPUTERROR or -EILSEQ.
| (since it's after feature-freeze :)
| And the original problem was that a leading '-' sign on a
| signed number (!) caused a return of 0.  At least that is fixed.
|
| So now the problem (?) is that a '-' sign on an unsigned number
| returns 0.  We can always add a big printk() there that
| something is foul.  Other ideas?

It's noteworthy that vsscanf() completely gives up on scanning
the rest of the input data at this point.  E.g.:
	count = sscanf (input, "%x %i %i", &level, &next, &other);
with <input> = "-42 -86 -99" gives up on "-42" and returns 0
(as number of items scanned/converted).

-- 
~Randy

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Henning Schmiedehausen]

Hi,

On Mon, 2002-11-11 at 04:05, Randy.Dunlap wrote:
> On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:
> 
> | "Randy.Dunlap" <rddunlap@osdl.org> writes:
> |
> | >+		digit = *str;
> | >+		if (is_sign && digit == '-')
> | >+			digit = *(str + 1);
> |
> | If signed is not allowed and you get a "-", you're in an error case
> | again...
> 
> Yes, and a 0 value is returned.
> IOW, asking for an unsigned number (in the format string)
> and getting "-123" does return 0.
> 
> What should it do?

I would model this after user space. (Which does strange things:

--- cut ---
#include <stdio.h>

main()
{
  char *scan = "-100";
  unsigned int foo;
  int bar;

  int res = sscanf(scan, "%ud", &foo);

  printf("%s = %ud = %d\n", scan, foo, res);

  res = sscanf(scan, "%ud", &bar);

  printf("%s = %d = %d\n", scan, bar, res);

}
--- cut ---

% gcc -o xxx xxx.c
./xxx 
-100 = 4294967196d = 1
-100 = -100 = 1

Hm, so I guess, yes, a warning message would be nice IMHO. Returning an
error code would IMHO moot, because noone is checking these codes
anyway.

	Regards
		Henning



> This function can't return -EINPUTERROR or -EILSEQ.
> (since it's after feature-freeze :)
> And the original problem was that a leading '-' sign on a
> signed number (!) caused a return of 0.  At least that is fixed.
> 
> So now the problem (?) is that a '-' sign on an unsigned number
> returns 0.  We can always add a big printk() there that
> something is foul.  Other ideas?


-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     hps@intermeta.de

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   info@intermeta.de
D-91054 Buckenhof     Fax.: 09131 / 50654-20   

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Andreas Schwab]

"Randy.Dunlap" <rddunlap@osdl.org> writes:

|> On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:
|> 
|> | "Randy.Dunlap" <rddunlap@osdl.org> writes:
|> |
|> | >+		digit = *str;
|> | >+		if (is_sign && digit == '-')
|> | >+			digit = *(str + 1);
|> |
|> | If signed is not allowed and you get a "-", you're in an error case
|> | again...
|> 
|> Yes, and a 0 value is returned.
|> IOW, asking for an unsigned number (in the format string)
|> and getting "-123" does return 0.

Not in C.  According to the standard scanf is supposed to convert the
value to unsinged and return that.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Re: sscanf("-1", "%d", &i) fails, returns 0

[Richard B. Johnson]

On Fri, 8 Nov 2002, Randy.Dunlap wrote:

> On Fri, 8 Nov 2002, Richard B. Johnson wrote:
> 
> | On Fri, 8 Nov 2002, Randy.Dunlap wrote:
> |
> | > On Sat, 9 Nov 2002, Douglas Gilbert wrote:
> | >
> | > | In lk 2.5.46-bk3 the expression in the subject line
> | > | fails to write into "i" and returns 0. Drop the minus
> | > | sign and it works.
> | >
> | > Here's an unobstrusive patch to correct that.
> | > Please apply.
> | >
> | > --
> | > ~Randy
> | > -
> |
> | I was thinking that if anybody ever had to change any of this
> | stuff, it might be a good idea to do the indirection only once?
> | All those "splats" over and over again are costly.
> 
> Sure, it looks cleaner that way, although gcc has already put <*dig>
> in a local register; i.e., it's not pulled from memory for each test.
> Here's a (tested) version that does that.
> 
> -- 
> ~Randy
> 
> 
> 
> --- ./lib/vsprintf.c%signed	Mon Nov  4 14:30:49 2002
> +++ ./lib/vsprintf.c	Fri Nov  8 12:03:15 2002
> @@ -517,6 +517,7 @@
>  {
>  	const char *str = buf;
>  	char *next;
> +	char *dig, onedig;
>  	int num = 0;
>  	int qualifier;
>  	int base;
> @@ -638,12 +639,14 @@
>  		while (isspace(*str))
>  			str++;
> 
> -		if (!*str
> -                    || (base == 16 && !isxdigit(*str))
> -                    || (base == 10 && !isdigit(*str))
> -                    || (base == 8 && (!isdigit(*str) || *str > '7'))
> -                    || (base == 0 && !isdigit(*str)))
> -			break;
> +		dig = (*str == '-') ? (str + 1) : str;
> +		onedig = *dig;
> +		if (!onedig
> +                    || (base == 16 && !isxdigit(onedig))
> +                    || (base == 10 && !isdigit(onedig))
> +                    || (base == 8 && (!isdigit(onedig) || onedig > '7'))
> +                    || (base == 0 && !isdigit(onedig)))
> +				break;
> 
>  		switch(qualifier) {
>  		case 'h':
> 

I like it much better.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).
   Bush : The Fourth Reich of America