Strange sscanf results

[Orfax]

I've been getting some strange sscanf results from ps2sdk (in libc.a), which I think is a bug.

The following snippet should return 3 (the amount of allocations done) but instead its returning 5 (the amount of conversions done).

Code: Select all

res = sscanf(str, "%s%d%d%*d%*c", s1, &n1, &n2);

I believe in xscanf.c in ps2sdk\ee\libc\src where it increments the nconvs after the nextconv tag it needs to check flags for not having FLSTAR set.

its probably easier to show it.
this needs to change at line 559...

Code: Select all

#if SCANF_LEVEL >= SCANF_FLT
    nextconv:
#endif
      flags = 0;
      if (clen > olen)
        nconvs++;
      else if (c != 'n' || i == EOF)
        /*
         * If one conversion failed completely,
         * punt.
         */
        goto leave;

to something like this...

Code: Select all

#if SCANF_LEVEL >= SCANF_FLT
    nextconv:
#endif
      if (clen > olen && !(flags & FLSTAR)) {
        flags = 0;
        nconvs++;
      }
      else if (c != 'n' || i == EOF) {
        /*
         * If one conversion failed completely,
         * punt.
         */
        flags = 0;
        goto leave;
      }
      else
        flags = 0;

I don't mind updating the repository if no one has any objections. If any one thinks it can be done better, comment away.

Orfax.

[Orfax]

I've been doing a bit more testing on the changes I did and had a problem caused by my changes.

The following code was finishing before it got to the fourth allocation.

Code: Select all

res = sscanf(str, "%s%d%d%*d%d%*c", s1, &n1, &n2, &n3);

To fix it I took out the format string character comparison against 'n'. I'm not sure why this was put there originally.

Code: Select all

else if (c != 'n' || i == EOF)

goes to

Code: Select all

else if (i == EOF)

The revised change should now look like:

Code: Select all

#if SCANF_LEVEL >= SCANF_FLT 
    nextconv: 
#endif 
      if (clen > olen && !(flags & FLSTAR)) { 
        flags = 0; 
        nconvs++; 
      } 
      else if (i == EOF) { 
        /* 
         * If one conversion failed completely, 
         * punt. 
         */ 
        flags = 0; 
        goto leave; 
      } 
      else 
        flags = 0;

On another matter, I also noticed fgets() doesn't leave the end of line marker at the end of the string returned (newline or carriage return, PS2 libc is using either as an end of line marker). gets() should remove it (though no one should be using this function), fgets() should leave it.

Orfax.

[Dr. Vegetable]

Is sscanf() supposed to terminate if it hits a newline? It almost looks like the comparison to 'n' should be '\n' ??!?

Code: Select all

else if (c != '\n' || i == EOF)

Just a guess...

Afterthought...
But then that also leaves me wondering why it wouldn't say:

Code: Select all

else if (c == '\n' || i == EOF)

But since I'm not actually looking at the code for the entire function, I should probably stop guessing.

[Orfax]

No, \n should just be treated as whitespace. At least it does under test programs I have for Cygwin, AIX, and PS2.

[Orfax]

Further testing has revealed a couple more issues with sscanf (and just as I was going to do a change on the repository). All these issues apply to all the scanf type functions BTW. I just happen to be using sscanf.

First issue, short ints were not working (ie "%hd"). This was because the flag define for a star and for short were set to the same value! The following needs to change (line 86 of xscanf.c):

Code: Select all

#define FLSHORT		0x10    /* arg is short int * */
#endif

#if SCANF_LEVEL >= SCANF_STD
#define FLSTAR		0x10	/* suppress assingment (* fmt) */
#endif

#if SCANF_LEVEL >= SCANF_FLT
#define FLBRACKET	0x20	/* %[ found, now collecting the set */
#define FLNEGATE	0x40	/* negate %[ set */

to this:

Code: Select all

#define FLSHORT		0x10    /* arg is short int * */
#endif

#if SCANF_LEVEL >= SCANF_STD
#define FLSTAR		0x20	/* suppress assingment (* fmt) */
#endif

#if SCANF_LEVEL >= SCANF_FLT
#define FLBRACKET	0x40	/* %[ found, now collecting the set */
#define FLNEGATE	0x80	/* negate %[ set */

Basically I've made FLSTAR 0x20 and pushed FLBRACKET and FLNEGATE out to higher values.




Second and more serious issue, I noticed sscanf was changing my string I was passing in. This bit of code illustrates it...

Code: Select all

static char *str	= "2  6";
int res		= 0;
short int n1	= 0;
short int n2	= 0;

n1 = n2 = 0;
printf("str=[%s] res=%d, n1=%d n2=%d str[4]=%02x",
	str, res, n1, n2, str[4]);
res = sscanf(str, "%hd%hd", &n1, &n2);
printf("str=[%s] res=%d, n1=%d n2=%d str[4]=%02x",
	str, res, n1, n2, str[4]);

After the call to sscanf the null character at the end of the string had been replaced with 0xFF.
The cause of this is the way the following #define is used through out the code.
#define w_xgetc(s) xgetc(s), clen++

Firstly, I have to say this define is nasty. N-A-S-T-Y.
When the function vxscanf (all the scanf functions are wrapper functions to this function) goes to read a character from the input stream, most of the time it does something like this:

Code: Select all

if ((i = w_xgetc(&stream)) == EOF)

What ends up happening with this is that "i" gets set to the character of the input stream, but EOF gets compared with clen! If an EOF had been read then it will pass through to the main processing for bits of the string, and may get put back on the input stream due to other safe guards. But for sscanf, it replaces the null character with the EOF, or more appropriately with EOF masked to a byte ie 0xFF.
Ideally the #define should be changed, but if we leave it the following should be changed. Where the above code gets used it should be changed to:

Code: Select all

i = w_xgetc(&stream);
if (i == EOF)

and the function _s_xungetc() should change from:

Code: Select all

void _s_xungetc(int c, void ** p_str) {
    *(--(*((char **)p_str))) = c;
}

to:

Code: Select all

void _s_xungetc(int c, void ** p_str) {
    *(--(*((char **)p_str))) = (c == EOF ? 0 : c);
}

which then makes it do the opposite of what _s_xgetc() does.

Anyway, I'll hang off a couple more days before doing the change to the repository. Just in case I find other things :)

[Orfax]

I've put my changes up on the SVN repository. Modified source files are stdio.c and xscanf.c in the libc library.

Orfax.