[PATCH 1/3] Change _offset type from int to _off_t

[Sebastian Huber]

On 11/23/2012 02:56 PM, Corinna Vinschen wrote:
> On Nov 23 14:26, Sebastian Huber wrote:
>> >On 11/23/2012 02:15 PM, Corinna Vinschen wrote:
>>> > >On Nov 23 13:24, Sebastian Huber wrote:
>>>> > >>On 11/23/2012 11:21 AM, Corinna Vinschen wrote:
>>>>> > >>>It breaks binary compatibility.  Full stop.  And this is a problem.
>>>> > >>
>>>> > >>Yes, this is a problem, but not for RTEMS.
>>>> > >>
>>>>> > >>>
>>>>> > >>>If we do that, we must make really sure that this isn't a problem.
>>>> > >>
>>>> > >>We can add a new type, which is int by default and for example _off_t on RTEMS.
>>> > >
>>> > >But this new type doesn't match your fseeko/ftello implementation
>>> > >anymore.
>> >
>> >Yes, this is bad.
> Note that I didn't say that the change from int to _off_t is wrong per
> se.  It just has to be considered very carefully.
>
> We get no problems at all for targets where sizeof(long) == sizeof(int).
> But we're in trouble as soon as that's not the case anymore.
>
> This affects 16 bit targets with typically sizeof(int) == 2 and
> sizeof(long) == 4 as well as all LP64 targets.
>
> But here's another problem:  For these targets with sizeof(int) <
> sizeof(long), the structure __sFILE was always(!) wrong, because the
> offset member was never able to cover the whole _off_t offset range.
>
> So, on second thought, it might be*necessary*  to change offset from
> int to _off_t, because that's a required bug fix.
>
> But then, how do we handle backward compatibility here?  I really
> don't know.
>
> Any suggestions?

Lets introduce

#if defined(__rtems__) || X
   typedef _off_t __sFile_off_t;
#else
   typedef int __sFile_off_t;
#endif

somewhere.

I assume _off_t is equal to _fpos_t.

How do my change to use ftello() and fseeko() work with
   sizeof(__sFile_off_t) <= sizeof(long) or
   sizeof(__sFile_off_t) <= sizeof(_off_t).

First ftello():

_off_t
_DEFUN(_ftello_r, (ptr, fp),
        struct _reent * ptr _AND
        register FILE * fp)
{
   _fpos_t pos;
[...]
   if (fp->_flags & __SOFF)
     pos = fp->_offset;
[...]
   return pos;
}

So this is not a problem.

Now fseeko():

int
_DEFUN(_fseeko_r, (ptr, fp, offset, whence),
        struct _reent *ptr _AND
        register FILE *fp  _AND
        _off_t offset      _AND
        int whence)
{
   _fpos_t _EXFNPTR(seekfn, (struct _reent *, _PTR, _fpos_t, int));
   _fpos_t target;
   _fpos_t curoff = 0;
[...]
   switch (whence)
     {
     case SEEK_CUR:
       /*
        * In order to seek relative to the current stream offset,
        * we have to first find the current stream offset a la
        * ftell (see ftell for details).
        */
       _fflush_r (ptr, fp);   /* may adjust seek offset on append stream */
       if (fp->_flags & __SOFF)
	curoff = fp->_offset;
[...]
   if (!havepos)
     {
       if (fp->_flags & __SOFF)
	curoff = fp->_offset;
[...]
}

So this is not a problem.

The problem is here (newlib/libc/stdio/fflush.c):

int
_DEFUN(__sflush_r, (ptr, fp),
        struct _reent *ptr _AND
        register FILE * fp)
{
[...]
#ifdef __LARGE64_FILES
	  _fpos64_t curoff;
#else
	  _fpos_t curoff;
#endif
[...]
	      if ((fp->_flags & __SOFF) && (curoff != -1 || ptr->_errno == 0))
		fp->_offset = curoff;
[...]
}

And here (newlib/libc/stdio/stdio.c):

_READ_WRITE_RETURN_TYPE
_DEFUN(__sread, (ptr, cookie, buf, n),
        struct _reent *ptr _AND
        void *cookie _AND
        char *buf _AND
        int n)
{
   register FILE *fp = (FILE *) cookie;
   register int ret;
[...]
   if (ret >= 0)
     fp->_offset += ret;
[...]
   return ret;
}

I think we have a bug here.  The
   "register int ret;" should be
   "register _READ_WRITE_RETURN_TYPE ret;".

_fpos_t
_DEFUN(__sseek, (ptr, cookie, offset, whence),
        struct _reent *ptr _AND
        void *cookie _AND
        _fpos_t offset _AND
        int whence)
{
   register FILE *fp = (FILE *) cookie;
   register _off_t ret;
[...]
       fp->_offset = ret;
     }
   return ret;
}

>
>>>>> > >>>In theory you're not supposed to change struct __sFILE, rather you're
>>>>> > >>>supposed to use struct __sFILE64 and the functions defined in the
>>>>> > >>>libc/stdio64 subdir.  This discussion seems to have gone lost.
>>>> > >>
>>>> > >>Ok, and how can I do this.  Is this a configure option?  I don't
>>>> > >>want to use fseeko64() directly.
>>> > >
>>> > >Why not?  Cygwin is doing the same.  The application calls fseeko,
>>> > >but internally Cygwin calls fseeko64 since the link lib redirects
>>> > >the call.  fseeko64 does not exist on the application level.
>> >
>> >Ok, the problem is that I am not a Newlib configuration guru.  How
>> >do I get this behaviour for RTEMS.  Is this a magic configure option
>> >or do I have to set some pre-processor defines somewhere?
> I don't know how to do that for RTEMS.  In Cygwin we have an import
> library which defines a symbol fseeko which is nothing but an assembler
> jmp to fseeko64.

Since there seem to be no ready to use solution I prefer to fix fseeko() and 
ftello().

-- 
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax     : +49 89 18 90 80 79-9
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.