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

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

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':

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':

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

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

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':

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

| "Randy.Dunlap" <[email protected]> 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

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

| On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:
|
| | "Randy.Dunlap" <[email protected]> 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

Hi,

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

Dipl.-Inf. (Univ.) Henning P. Schmiedehausen       -- Geschaeftsfuehrer
INTERMETA - Gesellschaft fuer Mehrwertdienste mbH     [email protected]

Am Schwabachgrund 22  Fon.: 09131 / 50654-0   [email protected]
D-91054 Buckenhof     Fax.: 09131 / 50654-20   

"Randy.Dunlap" <[email protected]> writes:

|> On Sun, 10 Nov 2002, Henning P. Schmiedehausen wrote:
|> 
|> | "Randy.Dunlap" <[email protected]> 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, [email protected]
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."