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

[Alan Hourihane]

On Sat, 2009-02-28 at 21:30 +0100, Bruno Haible wrote:
> Hi Alan,

Hi Bruno !

> > >   <http://savannah.gnu.org/patch/?6758>
> > >                  Summary: Add support for Atari FreeMiNT OS
> > >                  Project: GNU Core Utilities
> 
> Looked briefly at the stdio extensions functions, some things looked fishy:
>   - 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.

>   - In fseeko.c, an 'if (0)' that looks implausible (did you run the gnulib 
> unit
>     tests at all?),

Yes, I did run the tests. It falls back to fseeko(). It looks as though
it's possible to fallback to fseeko() in other cases too. But I can try
your addition and report back.

>   - half of the stdio extensions from gnulib were not ported at all.

I only sent the portions that related to coreutils 7.1 which is where I
put the original patch. There are others related to other functions too,
and I can submit them independently if required to the gnulib patch
tracker. But carry on reading to the very end of the email as I've made
comments on each item.

> So I did a port myself, based on the mintlib-0.58.0 sources. Afterwards, I 
> used
> 3 lines of code from your freadahead.c patch; I don't think you need to file
> copyright assignments merely for these 3 lines.
> 
> You can test the result by doing
>   $ ./gnulib-tool --create-testdir --dir=.../testdir-stdioext --with-tests \
>     fbufmode fflush fpending fpurge freadable freadahead freading freadptr 
> freadseek \
>     fseek fseeko fseterr ftell ftello fwritable fwriting 
> and in that directory:
>   $ ./configure CFLAGS=-g
>   $ make
>   $ make check
> 
> It is possible that there are problems with freading, fwriting, fpending. Let
> us know, please.

I don't think freading & fwriting will work I'm afraid. I don't see
anything here for fpending.

> Bruno
> 
> 
> 2009-02-28  Bruno Haible  <address@hidden>
> 
>       Add tentative support for FreeMiNT.
>       * lib/fbufmode.c (fbufmode) [__MINT__]: Add conditional code.
>       * lib/fflush.c (clear_ungetc_buffer): Likewise.
>       * lib/fpurge.c (fpurge): Likewise.
>       * lib/freadable.c (freadable): Likewise.
>       * lib/freading.c (freading): Likewise.
>       * lib/freadptr.c (freadptr): Likewise.
>       * lib/freadseek.c (freadptrinc): Likewise.
>       * lib/fseeko.c (rpl_fseeko): Likewise.
>       * lib/fseterr.c (fseterr): Likewise.
>       * lib/fwritable.c (fwritable): Likewise.
>       * lib/fwriting.c (fwriting): Likewise.
>       * lib/freadahead.c (freadahead): Likewise, based on code by Alan
>       Hourihane.
>       Reported by Alan Hourihane <address@hidden>.
> 
> --- lib/fbufmode.c.orig       2009-02-28 21:09:16.000000000 +0100
> +++ lib/fbufmode.c    2009-02-28 19:56:10.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
> @@ -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


>  #else
>   #error "Please port gnulib fbufmode.c to your platform! Look at the setvbuf 
> implementation."
>  #endif
> --- 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().

>  # else                              /* other implementations */
>    fseek (fp, 0, SEEK_CUR);
>  # endif
> --- lib/fpurge.c.orig 2009-02-28 21:09:16.000000000 +0100
> +++ lib/fpurge.c      2009-02-28 21:03:04.000000000 +0100
> @@ -1,5 +1,5 @@
>  /* Flushing buffers of 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
> @@ -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 ?

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

>  # else
>   #error "Please port gnulib fpurge.c to your platform! Look at the 
> definitions of fflush, setvbuf and ungetc on your system, then report this to 
> bug-gnulib."
>  # endif
> --- lib/freadable.c.orig      2009-02-28 21:09:16.000000000 +0100
> +++ lib/freadable.c   2009-02-28 19:48:54.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
> @@ -37,6 +37,8 @@
>    return (fp->_flag & (_IORW | _IOREAD)) != 0;
>  #elif defined __QNX__               /* QNX */
>    return (fp->_Mode & 0x1 /* _MOPENR */) != 0;
> +#elif defined __MINT__              /* Atari FreeMiNT */
> +  return fp->__mode.__read;
>  #else
>   #error "Please port gnulib freadable.c to your platform! Look at the 
> definition of fopen, fdopen on your system, then report this to bug-gnulib."
>  #endif
> --- lib/freadahead.c.orig     2009-02-28 21:09:16.000000000 +0100
> +++ lib/freadahead.c  2009-02-28 21:04:59.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
> @@ -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);


>  #elif defined SLOW_BUT_NO_HACKS     /* users can define this */
>    abort ();
>    return 0;
> --- lib/freading.c.orig       2009-02-28 21:09:16.000000000 +0100
> +++ lib/freading.c    2009-02-28 21:07:15.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
> @@ -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;
  return (fp->__flags & _IO_CURRENTLY_GETTING) != 0;


>   #error "Please port gnulib freading.c to your platform!"
>  #endif
> --- lib/freadptr.c.orig       2009-02-28 21:09:16.000000000 +0100
> +++ lib/freadptr.c    2009-02-28 20:52:40.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
> @@ -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;


>  #elif defined SLOW_BUT_NO_HACKS     /* users can define this */
>    /* This implementation is correct on any ANSI C platform.  It is just
>       awfully slow.  */
> --- lib/freadseek.c.orig      2009-02-28 21:09:16.000000000 +0100
> +++ lib/freadseek.c   2009-02-28 20:54:29.000000000 +0100
> @@ -1,5 +1,5 @@
>  /* Skipping input from 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
> @@ -53,6 +53,8 @@
>  # endif
>  #elif defined __QNX__               /* QNX */
>    fp->_Next += increment;
> +#elif defined __MINT__              /* Atari FreeMiNT */
> +  fp->__bufp += increment;
>  #elif defined SLOW_BUT_NO_HACKS     /* users can define this */
>  #else
>   #error "Please port gnulib freadseek.c to your platform! Look at the 
> definition of getc, getc_unlocked on your system, then report this to 
> bug-gnulib."
> --- lib/fseeko.c.orig 2009-02-28 21:09:16.000000000 +0100
> +++ lib/fseeko.c      2009-02-28 20:58:14.000000000 +0100
> @@ -82,6 +82,11 @@
>    if ((fp->_Mode & _MWRITE ? fp->_Next == fp->_Buf : fp->_Next == fp->_Rend)
>        && fp->_Rback == fp->_Back + sizeof (fp->_Back)
>        && fp->_Rsave == NULL)
> +#elif defined __MINT__              /* Atari FreeMiNT */
> +  if (fp->__bufp == fp->__buffer
> +      && fp->__get_limit == fp->__bufp
> +      && fp->__put_limit == fp->__bufp
> +      && !fp->__pushed_back)
>  #else

I can try this one.

>    #error "Please port gnulib fseeko.c to your platform! Look at the code in 
> fpurge.c, then report this to bug-gnulib."
>  #endif
> --- lib/fseterr.c.orig        2009-02-28 21:09:16.000000000 +0100
> +++ lib/fseterr.c     2009-02-28 19:45:56.000000000 +0100
> @@ -1,5 +1,5 @@
>  /* Set the error indicator of a 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
> @@ -41,6 +41,8 @@
>    fp->__modeflags |= __FLAG_ERROR;
>  #elif defined __QNX__               /* QNX */
>    fp->_Mode |= 0x200 /* _MERR */;
> +#elif defined __MINT__              /* Atari FreeMiNT */
> +  fp->__error = 1;
>  #elif 0                             /* unknown  */
>    /* Portable fallback, based on an idea by Rich Felker.
>       Wow! 6 system calls for something that is just a bit operation!
> --- lib/fwritable.c.orig      2009-02-28 21:09:16.000000000 +0100
> +++ lib/fwritable.c   2009-02-28 19:49:04.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
> @@ -37,6 +37,8 @@
>    return (fp->_flag & (_IORW | _IOWRT)) != 0;
>  #elif defined __QNX__               /* QNX */
>    return (fp->_Mode & 0x2 /* _MOPENW */) != 0;
> +#elif defined __MINT__              /* Atari FreeMiNT */
> +  return fp->__mode.__write;
>  #else
>   #error "Please port gnulib fwritable.c to your platform! Look at the 
> definition of fopen, fdopen on your system, then report this to bug-gnulib."
>  #endif
> --- 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;
  return (fp->__flags & _IO_CURRENTLY_PUTTING) != 0;

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

Thanks for the comments Bruno,

Alan.