forums.ps2dev.org Forum Index forums.ps2dev.org
Homebrew PS2, PSP & PS3 Development Discussions
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

fseek returns offset upon success instead of 0

 
Post new topic   Reply to topic    forums.ps2dev.org Forum Index -> PS2 Development
View previous topic :: View next topic  
Author Message
ragnarok2040



Joined: 09 Aug 2006
Posts: 230
PostPosted: Sat Jul 28, 2007 10:07 pm    Post subject: fseek returns offset upon success instead of 0 Reply with quote
I noticed this when I had fseek in an if statement, if fseek succeeds it is supposed to return 0 but it instead returns the offset of the file from the return of fioLseek. I fixed it by checking whether fioLseek returned with an error and setting ret to 0 if it had not. I'm not sure if that's the correct way to do it as I'm not familiar with fioLseek();, e.g. if it returns -1 when it doesn't succeed. This should fix fsetpos(); as well. I didn't have any problems building ps2sdk afterwards, but any code that relied on fseek returning an offset or code added afterwards that used fseek in a conditional statement might need fixing.

Edit:
I just took a look using codeblocks and there doesn't seem to be any code anywhere in ps2sdk that this fix would break. It should also fix the tests in test_fseek_ftell(); in stdio_tests.c.

More information:
http://www.opengroup.org/onlinepubs/007908775/xsh/fseek.html

Code:

int fseek(FILE *stream, long offset, int origin)
{
  int ret;
 
  stream->has_putback = 0;

  switch(stream->type) {
    case STD_IOBUF_TYPE_NONE:
    case STD_IOBUF_TYPE_GS:
    case STD_IOBUF_TYPE_SIO:
    case STD_IOBUF_TYPE_STDOUTHOST:
      /* cannot seek stdout or stderr. */
      ret = -1;
      break;
    default:
      /* attempt to seek to offset from origin. */
      ret = fioLseek(stream->fd, (int)offset, origin);
  }
  return (ret);
}

should be
Code:

int fseek(FILE *stream, long offset, int origin)
{
  int ret;
 
  stream->has_putback = 0;

  switch(stream->type) {
    case STD_IOBUF_TYPE_NONE:
    case STD_IOBUF_TYPE_GS:
    case STD_IOBUF_TYPE_SIO:
    case STD_IOBUF_TYPE_STDOUTHOST:
      /* cannot seek stdout or stderr. */
      ret = -1;
      break;
    default:
      /* attempt to seek to offset from origin. */
      if((fioLseek(stream->fd, (int)offset, origin)) != -1) {
        ret = 0;
      }
      else
        ret = -1;
  }
  return (ret);
}
Back to top
View user's profile Send private message
dlanor



Joined: 28 Oct 2004
Posts: 269
Location: Stockholm, Sweden
PostPosted: Sun Jul 29, 2007 1:37 am    Post subject: Re: fseek returns offset upon success instead of 0 Reply with quote
ragnarok2040 wrote:
I noticed this when I had fseek in an if statement, if fseek succeeds it is supposed to return 0 but it instead returns the offset of the file from the return of fioLseek. I fixed it by checking whether fioLseek returned with an error and setting ret to 0 if it had not. I'm not sure if that's the correct way to do it as I'm not familiar with fioLseek();, e.g. if it returns -1 when it doesn't succeed.
I'm not 100% sure what the formal definition says about this, as I don't have the relevant Sony documents. But even without knowing this you can still modify your patch to make it 100% sure of compatibility.

You just need to change part of your patch from:
Code:
      if((fioLseek(stream->fd, (int)offset, origin)) != -1) {
        ret = 0;
      }
      else
        ret = -1;


to:
Code:
      if((fioLseek(stream->fd, (int)offset, origin)) >= 0) {
        ret = 0;
      }
      else
        ret = -1;

Thus any positive return value from fioLseek gets changed to a zero, while any negative return value gets changed to -1, so it works regardless of whether fioLseek can return detailed error codes or just -1. I think this flexibility may be important, as generic use of fioLseek really implies calling various unrelated device drivers that may differ in this regard. PS2SDK itself has no direct control over that, as such a device driver might not even be related to PS2SDK in any way.

Quote:
but any code that relied on fseek returning an offset or code added afterwards that used fseek in a conditional statement might need fixing.
For non-error returns I don't see that as a problem, since any code usíng the standard C fseek function must also expect its standard return values. If it doesn't, then it is that code which is incorrect, and then it needs fixing on general principles anyway. The ANSI definition of fseek clearly states that the non-error return value should be zero and nothing else.

Interestingly enough, the same definition does NOT state that the return value for errors must be -1. Instead it simply states that the return value for those cases will be non-zero. This apparently implies that any non-zero values would be acceptable, so that we might as well pass on the PS2-style error codes (if any) from fioLseek.

For such implementation the critical part of the patch could then become:
Code:
      ret = fioLseek(stream->fd, (int)offset, origin);
      if(ret >= 0)
        ret = 0;


This should fix the problem you ran into, just like your patch, while still holding a door open for detailed error return codes.

Best regards: dlanor
Back to top
View user's profile Send private message
ragnarok2040



Joined: 09 Aug 2006
Posts: 230
PostPosted: Sun Jul 29, 2007 2:27 am    Post subject: Reply with quote
Nice, thanks dlanor. I created a patch from the implementation you suggested.
Code:

diff -burN orig.ps2sdk/ee/libc/src/stdio.c ps2sdk/ee/libc/src/stdio.c
--- orig.ps2sdk/ee/libc/src/stdio.c   Sat Jul 28 12:00:45 2007
+++ ps2sdk/ee/libc/src/stdio.c   Sat Jul 28 12:11:54 2007
@@ -781,6 +781,8 @@
     default:
       /* attempt to seek to offset from origin. */
       ret = fioLseek(stream->fd, (int)offset, origin);
+      if (ret >= 0)
+        ret = 0;
   }
   return (ret);
 }
Back to top
View user's profile Send private message
ooPo
Site Admin


Joined: 17 Jan 2004
Posts: 2032
Location: Canada
PostPosted: Sun Jul 29, 2007 12:26 pm    Post subject: Reply with quote
Code:
Sending        ee/libc/src/stdio.c
Transmitting file data .
Committed revision 1429.

Added to the repository.
Back to top
View user's profile Send private message Visit poster's website
chp



Joined: 23 Jun 2004
Posts: 313
PostPosted: Mon Jul 30, 2007 6:14 pm    Post subject: Reply with quote
The lowlevel function returns a negative representation of what should be put in the errno variable when it fails (for example -EBADF). If you want to properly shape this up, you should also store that value in errno (converted into a positive value first of course) before returning -1.
_________________
GE Dominator
Back to top
View user's profile Send private message
ragnarok2040



Joined: 09 Aug 2006
Posts: 230
PostPosted: Mon Jul 30, 2007 8:42 pm    Post subject: Reply with quote
Ahh, I see. I modified the fix so that it does that. It's kind of redundant now to return fseek with the negative errno value, but that shouldn't matter, heh.
Code:

      ret = fioLseek(stream->fd, (int)offset, origin);
      if (ret < 0)
        errno = ret * (-1);
      else
        ret = 0;
Back to top
View user's profile Send private message
Display posts from previous:   
Post new topic   Reply to topic    forums.ps2dev.org Forum Index -> PS2 Development All times are GMT + 10 Hours
Page 1 of 1

 
Jump to:  
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