 |
forums.ps2dev.org
Homebrew PS2, PSP & PS3 Development Discussions
|
| View previous topic :: View next topic |
| Author |
Message |
SSpeare
Joined: 23 May 2006
Posts: 63
|
 Posted: Wed Jun 14, 2006 9:45 pm Post subject: fread and ungetc bug fix |
 |
|
While working on lua support we discovered what appears to be a bug in stdio.c. The data that is putback on the stream by ungetc() is not honored by some other functions. Notably, fread() will ignore the putback which breaks lua file loading. We're proposing the below fix. Any input would be appreciated:
| Code: | size_t fread(void *buf, size_t r, size_t n, FILE *stream)
{
size_t ret;
switch(stream->type) {
case STD_IOBUF_TYPE_NONE:
case STD_IOBUF_TYPE_GS:
case STD_IOBUF_TYPE_SIO:
case STD_IOBUF_TYPE_STDOUTHOST:
/* cannot read from stdout or stderr. */
ret = 0;
break;
default:
/* attempt to read from the stream file. */
if (stream->has_putback) {
buf[0] = stream->putback;
buf++;
stream->has_putback = 0;
/* subtract 1 from r * n to avoid buffer overflow */
ret = (_ps2sdk_read(stream->fd, buf, (int)((r * n) -1 ))) / (int)r);
} else {
ret = (_ps2sdk_read(stream->fd, buf, (int)(r * n)) / (int)r);
}
}
return (ret);
} |
Something else I noticed:
fread() will return partial records if your record size is >1. That seems odd to me, but that is the current behavior so I guess we should just leave it alone for the moment.
e.g.
If I ask to read 10 records of size 4, and fread() finds 38 bytes, I would expect it to return 9 records of size 4 (36 bytes), not 38 bytes which is 9 records plus a half record. The return value of fread() will be 9 because it is forced to an int, so it will lie about how much data it read. Then if you fread() again you will have mis-aligned data because you have already read 1/2 of the record, but you didn't know that because the function lied.
I'm not sure what the behavior of fread() is in a standard implementation, though... I haven't used C extensively in a long time. This may be just a caveat.
It should still probably subtract the 1 from (r * n)... Either way it won't fix the above problem. |
|
| Back to top |
|
 |
evilo
Joined: 22 Apr 2004
Posts: 230
|
| Posted: Wed Jun 14, 2006 9:49 pm Post subject: |
|
|
| Furthermore, as also said previously, the stream->as_putback flag is not checked (where it should be) in fwrite, ftell, fgetpos and not reseted in some others (like fsetpos, etc..) |
|
| Back to top |
|
|
evilo
Joined: 22 Apr 2004
Posts: 230
|
| Posted: Thu Jun 15, 2006 4:14 am Post subject: |
|
|
so this is the final working correction
| Code: |
size_t fread(void *buf, size_t r, size_t n, FILE *stream)
{
size_t ret;
switch(stream->type) {
case STD_IOBUF_TYPE_NONE:
case STD_IOBUF_TYPE_GS:
case STD_IOBUF_TYPE_SIO:
case STD_IOBUF_TYPE_STDOUTHOST:
/* cannot read from stdout or stderr. */
ret = 0;
break;
default:
/* attempt to read from the stream file. */
if (stream->has_putback) {
unsigned char *ptr = (unsigned char *)buf;
*ptr = stream->putback;
buf = ptr + 1;
stream->has_putback = 0;
/* subtract 1 from r * n to avoid buffer overflow */
ret = (_ps2sdk_read(stream->fd, buf, (int)((r * n) -1 )) / (int)r);
} else {
ret = (_ps2sdk_read(stream->fd, buf, (int)(r * n)) / (int)r);
}
}
return (ret);
}
|
however, when running the regression tests, fgets seems to be broken, I dunno if it's linked since i didn't ran it before my modification. i'll put the update in svn once fixed.
cheers.
[EDIT] forgot to mention that the blank line at the start of a lua script is not required anymore :) |
|
| Back to top |
|
|
evilo
Joined: 22 Apr 2004
Posts: 230
|
| Posted: Thu Jun 15, 2006 8:19 am Post subject: |
|
|
fgets function seems to be fixed even if the following test (test #2 from the regression tests) always fail..
| Code: |
ret = fgets(buf, sizeof(buf), fp);
fclose(fp);
if (ret!=buf || strcmp(buf, "hello world\n") != 0)
{
return "read wrong data from file";
}
|
but if buf and ret are printed, they are both equals...
anyway I still have an issue as well with ftell/fseek test (test #5 of the regression test) but since I'm too tired, i'm stoping for today ...
here is the current changes I made to libc, if you want to have a look (in case you see how to correct it (run the regression test to see it) :
http://psxdev.info/evilo/download/libc/src/stdio.c
evilo
evilo. |
|
| Back to top |
|
|
Orfax
Joined: 08 Apr 2004
Posts: 21
Location: Adelaide, Australia
|
| Posted: Thu Jun 15, 2006 11:11 am Post subject: Re: fread and ungetc bug fix |
|
|
| SSpeare wrote: | | While working on lua support we discovered what appears to be a bug in stdio.c. |
Yes I think there are a few more still (I will have a look at those when I get around to them).
While it may have nothing to do with what you guys have found, I just wanted to let you know I recently did some changes to the libc (and stdio.c). I want to make sure the changes I have done, haven't caused some side effects (I don't think they have, these are new issues).
I *have* modified fgets().
| SSpeare wrote: | | I'm not sure what the behavior of fread() |
In the man page (on cygwin) it just says fread returns the number of elements it successfully reads. You may want to set up a test case showing the behaviour and try it on a number of different fread implementations to see what is returned (I tend to use ps2dev, cygwin and AIX because they are what I have access to).
| evilo wrote: | | but if buf and ret are printed, they are both equals... |
fgets returns either the buffer passed in (if values were read) or NULL (if it was not able to read anything). fgets() was not putting the newline in the buffer when it was read. This is what I fixed with it. I did not change the regression tests (and I really should have). Is this why the regression test is failing?
| evilo wrote: | | Furthermore, as also said previously, the stream->as_putback flag is not checked |
Also please check the scanf type functions. They also put a character back onto the stream. I had the issue where they were putting the wrong character back.
For more information about what I did please see the following topic:
http://forums.ps2dev.org/viewtopic.php?t=4158
_________________
Look deep into their eyes for what they have to say, Emotions take control of life everyday
Death (Nothing Is Everything) |
|
| Back to top |
|
|
evilo
Joined: 22 Apr 2004
Posts: 230
|
| Posted: Fri Jun 16, 2006 5:10 am Post subject: |
|
|
Changes are commited into svn . I checked scanf and it seems ok for me, though I didn't tested to be honest.
evilo |
|
| 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
