 |
forums.ps2dev.org
Homebrew PS2, PSP & PS3 Development Discussions
|
| View previous topic :: View next topic |
| Author |
Message |
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
 Posted: Sun Nov 20, 2005 11:03 pm Post subject: Strange sscanf results |
 |
|
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: | | 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: |
#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: |
#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. |
|
| Back to top |
|
 |
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
| Posted: Wed Nov 23, 2005 2:34 pm Post subject: |
|
|
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: | | 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: | | else if (c != 'n' || i == EOF) |
goes to
The revised change should now look like:
| Code: | #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. |
|
| Back to top |
|
|
Dr. Vegetable
Joined: 14 Nov 2005
Posts: 171
Location: Boston, Massachusetts
|
| Posted: Fri Nov 25, 2005 11:33 am Post subject: |
|
|
Is sscanf() supposed to terminate if it hits a newline? It almost looks like the comparison to 'n' should be '\n' ??!?
| Code: | | else if (c != '\n' || i == EOF) |
Just a guess...
Afterthought...
But then that also leaves me wondering why it wouldn't say:
| Code: | | 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. |
|
| Back to top |
|
|
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
| Posted: Fri Nov 25, 2005 3:50 pm Post subject: |
|
|
| No, \n should just be treated as whitespace. At least it does under test programs I have for Cygwin, AIX, and PS2. |
|
| Back to top |
|
|
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
| Posted: Sun Dec 04, 2005 1:15 am Post subject: |
|
|
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: |
#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: |
#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: |
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: | | 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: |
i = w_xgetc(&stream);
if (i == EOF) |
and the function _s_xungetc() should change from:
| Code: | void _s_xungetc(int c, void ** p_str) {
*(--(*((char **)p_str))) = c;
}
|
to:
| Code: | 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 :) |
|
| Back to top |
|
|
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
| Posted: Wed Feb 22, 2006 2:14 am Post subject: |
|
|
I've put my changes up on the SVN repository. Modified source files are stdio.c and xscanf.c in the libc library.
Orfax.
_________________
Look deep into their eyes for what they have to say, Emotions take control of life everyday
Death (Nothing Is Everything) |
|
| Back to top |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
