Re: [patch #6758] Add support for Atari FreeMiNT OS

[Bruno Haible]

Hi Alan,

> I see this has already landed in git, so let me know how you want to
> handle any updates.

Can you please take the current gnulib (git HEAD), execute the tests in
the way I mentioned, show us the test failures (with line numbers of the
failing ASSERT) and then put in the minimal set of modifications that gets
the failures fixed?

> >   - In freading.c, the use of a field __flags and a macro 
> > _IO_CURRENTLY_GETTING
> >     which are not defined in mintlib-0.58.0. This cannot compile?!
> 
> That new handling is in the CVS for mintlib.

OK, so let's use these __flags if and only if necessary to fix a test failure.

> > --- lib/fbufmode.c.orig     2009-02-28 21:09:16.000000000 +0100
> > +++ lib/fbufmode.c  2009-02-28 19:56:10.000000000 +0100
> > @@ -73,6 +73,10 @@
> >    if (fp->_Mode & 0x800 /* _MNBF */)
> >      return _IONBF;
> >    return _IOFBF;
> > +#elif defined __MINT__              /* Atari FreeMiNT */
> > +  if (fp->__linebuf)
> > +    return _IOLBF;
> > +  return (fp->__bufsize > 0 ? _IOFBF : _IONBF);
> 
> This can be done like this...
> 
> #elif defined __MINT__
>   if (!fp->__buffer)
>     return _IONBF;
>   if (fp->__linebuf)
>     return _IOLBF;
>   return _IOFBF;
> #else

Yes, both codes appear to be equivalent (looking at
mintlib-0.58.0/stdio/setvbuf.c).

> > --- lib/fflush.c.orig       2009-02-28 21:09:16.000000000 +0100
> > +++ lib/fflush.c    2009-02-28 21:01:40.000000000 +0100
> > @@ -63,6 +63,12 @@
> >      }
> >  # elif defined _IOERR               /* AIX, HP-UX, IRIX, OSF/1, Solaris, 
> > OpenServer, mingw */
> >    /* Nothing to do.  */
> > +# elif defined __MINT__             /* Atari FreeMiNT */
> > +  if (fp->__pushed_back)
> > +    {
> > +      fp->__bufp = fp->__pushback_bufp;
> > +      fp->__pushed_back = 0;
> > +    }
> 
> Don't need this - we already have fflush().

It may become used if we add even stricter tests to test-fflush.c and 
m4/fflush.m4.

> > --- lib/fpurge.c.orig       2009-02-28 21:09:16.000000000 +0100
> > +++ lib/fpurge.c    2009-02-28 21:03:04.000000000 +0100
> > @@ -114,6 +114,16 @@
> >      /* fp->_Buf <= fp->_Next <= fp->_Rend */
> >      fp->_Rend = fp->_Next;
> >    return 0;
> > +# elif defined __MINT__             /* Atari FreeMiNT */
> > +  if (fp->__pushed_back)
> > +    {
> > +      fp->__bufp = fp->__pushback_bufp;
> > +      fp->__pushed_back = 0;
> > +    }
> > +  fp->__bufp = fp->__buffer;
> > +  fp->__get_limit = fp->__bufp;
> > +  fp->__put_limit = fp->__bufp;
> > +  return 0;
> 
> Why check for if (fp->__pushed_back) and then update fp->__bufp, we're
> going to overwrite it anyway ?

It's there for consistency with lib/fflush.c. I hope a good enough C compiler
will do the optimizations that you mention :-)

> I also think the reset logic is incorrect, You are resetting the buffer
> to the very start and both the read/write pointers follow. 
> 
> I do this in mine and it's passes the fpurge() tests...
> 
> # elif defined __MINT__
>   fp->__pushback_bufp = 0;
>   if (fp->__mode.__write)
>     fp->__put_limit = fp->__buffer;
>   fp->__bufp = fp->__get_limit;
>   return 0;
> # else

It's possible that my code is wrong. Let's use the unit tests as arbiter.

> > --- lib/freadahead.c.orig   2009-02-28 21:09:16.000000000 +0100
> > +++ lib/freadahead.c        2009-02-28 21:04:59.000000000 +0100
> > @@ -70,6 +70,12 @@
> >      + (fp->_Mode & 0x4000 /* _MBYTE */
> >         ? (fp->_Back + sizeof (fp->_Back)) - fp->_Rback
> >         : 0);
> > +#elif defined __MINT__              /* Atari FreeMiNT */
> > +  if (!fp->__mode.__read)
> > +    return 0;
> > +  return (fp->__pushed_back
> > +     ? fp->__get_limit - fp->__pushback_bufp + 1
> > +     : fp->__get_limit - fp->__bufp);
> 
> I do this...
> 
> #elif defined __MINT__
>   if (fp->__mode.__write)
>     return 0;
>   if (fp->__pushed_back)
>     return (fp->__get_limit - fp->__pushback_bufp + 1);
>   return (fp->__get_limit - fp->__bufp);

> > --- lib/freadptr.c.orig     2009-02-28 21:09:16.000000000 +0100
> > +++ lib/freadptr.c  2009-02-28 20:52:40.000000000 +0100
> > @@ -85,6 +85,14 @@
> >      return NULL;
> >    *sizep = size;
> >    return (const char *) fp->_Next;
> > +#elif defined __MINT__              /* Atari FreeMiNT */
> > +  if (!fp->__mode.__read)
> > +    return NULL;
> > +  size = fp->__get_limit - fp->__bufp;
> > +  if (size == 0)
> > +    return NULL;
> > +  *sizep = size;
> > +  return fp->__bufp;
> 
> And this...
> 
> #elif defined __MINT__
>   if (fp->__mode.__write)
>     return NULL;
>   size = fp->__get_limit - fp->__bufp;
>   if (size == 0)
>     return NULL;
>   *sizep = size;
>   return (const char *) fp->__bufp;

The freadahead and freadptr functions are supposed to work also for
read-write streams. They have fp->__mode.__write == 1 and
fp->__mode.__read == 1. Only on write-only streams should freadptr always
return NULL.

> > --- lib/freading.c.orig     2009-02-28 21:09:16.000000000 +0100
> > +++ lib/freading.c  2009-02-28 21:07:15.000000000 +0100
> > @@ -46,6 +46,11 @@
> >  #elif defined __QNX__               /* QNX */
> >    return ((fp->_Mode & 0x2 /* _MOPENW */) == 0
> >       || (fp->_Mode & 0x1000 /* _MREAD */) != 0);
> > +#elif defined __MINT__              /* Atari FreeMiNT */
> > +  return (!fp->__mode.__write
> > +     || (fp->__mode.__read
> > +         && (fp->__buffer < fp->__get_limit
> > +             /*|| fp->__bufp == fp->__put_limit ??*/)));
> >  #else
> 
> And this...
> 
> #elif defined __MINT__
>   if (!fp->__mode.__write && fp->__mode.__read)
>     return 1;

This is equivalent is mine: If fp->__mode.__write == 0, then surely
fp->__mode.__read == 1.

>   return (fp->__flags & _IO_CURRENTLY_GETTING) != 0;

And this one, let's again see in the unit tests.

> > --- lib/fwriting.c.orig     2009-02-28 21:09:16.000000000 +0100
> > +++ lib/fwriting.c  2009-02-28 21:07:21.000000000 +0100
> > @@ -1,5 +1,5 @@
> >  /* Retrieve information about a FILE stream.
> > -   Copyright (C) 2007-2008 Free Software Foundation, Inc.
> > +   Copyright (C) 2007-2009 Free Software Foundation, Inc.
> >  
> >     This program is free software: you can redistribute it and/or modify
> >     it under the terms of the GNU General Public License as published by
> > @@ -40,6 +40,11 @@
> >  #elif defined __QNX__               /* QNX */
> >    return ((fp->_Mode & 0x1 /* _MOPENR */) == 0
> >       || (fp->_Mode & 0x2000 /* _MWRITE */) != 0);
> > +#elif defined __MINT__              /* Atari FreeMiNT */
> > +  return (!fp->__mode.__read
> > +     || (fp->__mode.__write
> > +         && (fp->__buffer < fp->__put_limit
> > +             /*|| fp->__bufp == fp->__get_limit ??*/)));
> 
> And this...
> 
> #elif defined __MINT__
>   if (!fp->__mode.__read && fp->__mode.__write)
>     return 1;

Again, this 'if' condition is equivalent to 'if (!fp->__mode.__read)'.

>   return (fp->__flags & _IO_CURRENTLY_PUTTING) != 0;

As above, let's see whether this is needed for the unit tests to work.

Bruno