This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]