[PATCH] DragonFly BSD support

[Hasso Tepper-3]

The attached patch fixes DragonFly BSD support in gnulib. The patch is
written by Matthew Dillon and is tested by many users as part of GNU m4
pkgsrc package.


regards,

--
Hasso Tepper

[attachment removed]

[Bruno Haible]

Hi,

Hasso Tepper wrote:
> The attached patch fixes DragonFly BSD support in gnulib. The patch is
> written by Matthew Dillon and is tested by many users as part of GNU m4
> pkgsrc package.

Thank you for reporting this to us. We don't have the habit of checking the
various "ports collections" regularly.

Now, the first two hunks of your patch are nops: there are already
#ifdef __DragonFly__ tests further up in the two files.

About the third hunk, Eric Blake asked a for a rationale in
<http://lists.gnu.org/archive/html/bug-gnulib/2008-04/msg00269.html>
and got no reply.

If you want to help getting gnulib work on DragonFly:

You find in   http://www.haible.de/bruno/gnu/testdir-stdioext.tar.gz
the documentation and implementation for all systems so far (from BSD
systems to QNX), and a test suite. The documentation/specification is
in the .h files.

To port this package:

  - Unpack the package, configure it. Try "make" and fix the problems.
  - Run "make check" to see the test results.
  - The recommended order of porting (from easy to harder) is:
    fbufmode, fseterr, freadable, fwritable, freading, fwriting,
    fpending, fpurge, freadptr, freadseek, freadahead.

If you want to have the complete liberty to shuffle around fields in the
FILE struct, without risking crashes of programs that use gnulib, and you
add such primitive functions to DragonFly, gnulib will use these primitives
(provided they pass the testsuite).

Bruno

[VOROSKOI Andras]

Hi,

I've updated the original patch a bit. I've reworked the #ifdef
__DragonFly__ parts to avoid duplicates.

About the third hunk: this new patch does not touch that part.

Let me know if you still got problems with it.

PS: please cc me, i'm off-list

--
voroskoi

diff --git a/lib/freadahead.c b/lib/freadahead.c
index 610de87..8b790f4 100644
--- a/lib/freadahead.c
+++ b/lib/freadahead.c
@@ -30,11 +30,15 @@ freadahead (FILE *fp)
   return (fp->_IO_read_end - fp->_IO_read_ptr)
  + (fp->_flags & _IO_IN_BACKUP ? fp->_IO_save_end - fp->_IO_save_base :
     0);
-#elif defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
+#elif defined __sferror /* FreeBSD, NetBSD, OpenBSD, MacOS X, Cygwin */
   if ((fp_->_flags & __SWR) != 0 || fp_->_r < 0)
     return 0;
   return fp_->_r
  + (HASUB (fp) ? fp_->_ur : 0);
+#elif defined __DragonFly__
+  if ((fp_->_flags & __SWR) != 0 || fp_->_r < 0)
+  return 0;
+  return __sreadahead(fp);
 #elif defined __EMX__               /* emx+gcc */
   if ((fp->_flags & _IOWRT) != 0)
     return 0;
diff --git a/lib/freading.c b/lib/freading.c
index e201b89..9d9ffed 100644
--- a/lib/freading.c
+++ b/lib/freading.c
@@ -35,8 +35,10 @@ freading (FILE *fp)
   return ((fp->_flags & _IO_NO_WRITES) != 0
   || ((fp->_flags & (_IO_NO_READS | _IO_CURRENTLY_PUTTING)) == 0
       && fp->_IO_read_base != NULL));
-#elif defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
+#elif defined __sferror /* FreeBSD, NetBSD, OpenBSD, MacOS X, Cygwin */
   return (fp_->_flags & __SRD) != 0;
+#elif defined __DragonFly__
+  return ((((struct __FILE_public *)fp)->_flags & __SRD) != 0);
 #elif defined __EMX__               /* emx+gcc */
   return (fp->_flags & _IOREAD) != 0;
 #elif defined _IOERR                /* AIX, HP-UX, IRIX, OSF/1, Solaris, OpenServer, mingw */
diff --git a/lib/fseeko.c b/lib/fseeko.c
index 536dfaf..c6aa4fe 100644
--- a/lib/fseeko.c
+++ b/lib/fseeko.c
@@ -45,7 +45,7 @@ rpl_fseeko (FILE *fp, off_t offset, int whence)
   if (fp->_IO_read_end == fp->_IO_read_ptr
       && fp->_IO_write_ptr == fp->_IO_write_base
       && fp->_IO_save_base == NULL)
-#elif defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
+#elif defined __sferror /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
 # if defined __SL64 && defined __SCLE /* Cygwin */
   if ((fp->_flags & __SL64) == 0)
     {
@@ -65,6 +65,9 @@ rpl_fseeko (FILE *fp, off_t offset, int whence)
      ? fp_->_bf._size
      : 0)
       && fp_ub._base == NULL)
+#elif __DragonFly__
+  if (0)
+  /* NOTREACHED */
 #elif defined __EMX__               /* emx+gcc */
   if (fp->_ptr == fp->_buffer
       && fp->_rcount == 0
@@ -89,14 +92,14 @@ rpl_fseeko (FILE *fp, off_t offset, int whence)
       off_t pos = lseek (fileno (fp), offset, whence);
       if (pos == -1)
  {
-#if defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
+#if defined __sferror /* FreeBSD, NetBSD, OpenBSD, MacOS X, Cygwin */
   fp_->_flags &= ~__SOFF;
 #endif
   return -1;
  }
       else
  {
-#if defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
+#if defined __sferror /* FreeBSD, NetBSD, OpenBSD, MacOS X, Cygwin */
   fp_->_offset = pos;
   fp_->_flags |= __SOFF;
   fp_->_flags &= ~__SEOF;

[Bruno Haible]

VOROSKOI Andras wrote:
> I've updated the original patch a bit. I've reworked the #ifdef
> __DragonFly__ parts to avoid duplicates.
>
> About the third hunk: this new patch does not touch that part.

The first hunk (file freadahead.c) is correct when looking at the DragonFlyBSD
CVS
  http://www.dragonflybsd.org/cvsweb/src/include/stdio.h.diff?r1=1.12&r2=1.13
  http://www.dragonflybsd.org/cvsweb/src/lib/libc/stdio/sreadahead.c?rev=1.1&content-type=text/x-cvsweb-markup
I applied that.

The second hunk (file freading.c) appears to be a nop to me; why should I apply
this?
The third hunk (file fseeko.c) is lacking a rationale.

More generally, what were the problems that you encountered when you ran
"./configure; make; make check" in the testdir-stdioext.tar.gz that I posted?

Bruno



2008-06-01  Bruno Haible  <bruno@...>

        * lib/freadahead.c (freadahead) [__DragonFly__]: Use the __sreadahead
        function.
        Reported by VOROSKOI Andras <voroskoi@...>.

*** lib/freadahead.c.orig 2008-06-01 18:34:45.000000000 +0200
--- lib/freadahead.c 2008-06-01 18:29:47.000000000 +0200
***************
*** 33,40 ****
--- 33,44 ----
  #elif defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
    if ((fp_->_flags & __SWR) != 0 || fp_->_r < 0)
      return 0;
+ # if defined __DragonFly__
+   return __sreadahead (fp);
+ # else
    return fp_->_r
  + (HASUB (fp) ? fp_->_ur : 0);
+ # endif
  #elif defined __EMX__               /* emx+gcc */
    if ((fp->_flags & _IOWRT) != 0)
      return 0;

[VOROSKOI Andras]

On Sun, Jun 01, 2008 at 06:43:19PM +0200, Bruno Haible wrote:
> The first hunk (file freadahead.c) is correct when looking at the DragonFlyBSD
> I applied that.

Thanks.

> The second hunk (file freading.c) appears to be a nop to me; why should I apply
> this?

Well, skip that. testdir-stdioext compiles without that, but the test is
skipped. Is that normal?

> The third hunk (file fseeko.c) is lacking a rationale.

OK. If we can solve the compile problem mentioned later that's unnecessary.

> More generally, what were the problems that you encountered when you ran
> "./configure; make; make check" in the testdir-stdioext.tar.gz that I posted?

First I've applied this patch:

diff -aur testdir-stdioext/gllib/freadahead.c testdir-stdioext-dfly/gllib/freadahead.c
--- testdir-stdioext/gllib/freadahead.c 2008-05-01 16:38:47 +0200
+++ testdir-stdioext-dfly/gllib/freadahead.c    2008-06-02 00:35:43 +0200
@@ -33,8 +33,12 @@
 #elif defined __sferror || defined __DragonFly__ /* FreeBSD, NetBSD, OpenBSD, DragonFly, MacOS X, Cygwin */
   if ((fp_->_flags & __SWR) != 0 || fp_->_r < 0)
     return 0;
+# if defined __DragonFly__
+  return __sreadahead (fp);
+# else
   return fp_->_r
         + (HASUB (fp) ? fp_->_ur : 0);
+# endif
 #elif defined __EMX__               /* emx+gcc */
   if ((fp->_flags & _IOWRT) != 0)
     return 0;
diff -aur testdir-stdioext/gllib/stdio-impl.h testdir-stdioext-dfly/gllib/stdio-impl.h
--- testdir-stdioext/gllib/stdio-impl.h 2008-05-01 16:38:47 +0200
+++ testdir-stdioext-dfly/gllib/stdio-impl.h    2008-06-01 22:52:47 +0200
@@ -35,7 +35,7 @@
                         struct { unsigned char *_base; int _size; } _ub; \
                         int _ur; \
                         unsigned char _ubuf[3]; \
-                        unsigned cahr _nbuf[1]; \
+                        unsigned char _nbuf[1]; \
                         struct { unsigned char *_base; int _size; } _lb; \
                         int _blksize; \
                         fpos_t _offset; \


Then I got these errors:
fflush.c:38: error: dereferencing pointer to incomplete type
fseeko.c:67: error: dereferencing pointer to incomplete type

So there is some problem with
#define fp_ub fp->_ub
line on DragonFly, but I have no clue what to do with that.

If I workaround those make check's output is the following:
voroskoi ~/temp/testdir-stdioext-dfly $ make check
Making check in gllib
make  check-am
Making check in glm4
Making check in gltests
make  check-recursive
make  check-TESTS
PASS: test-EOVERFLOW
PASS: test-binary-io.sh
PASS: test-fbufmode
File offset is wrong after fseek: 17.
FAIL: test-fflush
PASS: test-fflush2.sh
PASS: test-fpending.sh
PASS: test-fpurge
Skipping test: file operations failed.
SKIP: test-freadable
PASS: test-freadahead.sh
Skipping test: file operations failed.
SKIP: test-freading
PASS: test-freadptr.sh
PASS: test-freadptr2.sh
PASS: test-freadseek.sh
PASS: test-fseek.sh
PASS: test-fseeko.sh
PASS: test-fseterr
PASS: test-ftell.sh
PASS: test-ftello.sh
Skipping test: file operations failed.
SKIP: test-fwritable
Skipping test: file operations failed.
SKIP: test-fwriting
test-lseek.c:49: assertion failed
Abort trap (core dumped)
FAIL: test-lseek.sh
PASS: test-stdbool
PASS: test-stdio
PASS: test-unistd
======================
2 of 20 tests failed
(4 tests were not run)
======================

--
voroskoi

[Bruno Haible]

VOROSKOI Andras wrote:

> diff -aur testdir-stdioext/gllib/stdio-impl.h testdir-stdioext-dfly/gllib/stdio-impl.h
> --- testdir-stdioext/gllib/stdio-impl.h 2008-05-01 16:38:47 +0200
> +++ testdir-stdioext-dfly/gllib/stdio-impl.h    2008-06-01 22:52:47 +0200
> @@ -35,7 +35,7 @@
>                          struct { unsigned char *_base; int _size; } _ub; \
>                          int _ur; \
>                          unsigned char _ubuf[3]; \
> -                        unsigned cahr _nbuf[1]; \
> +                        unsigned char _nbuf[1]; \
>                          struct { unsigned char *_base; int _size; } _lb; \
>                          int _blksize; \
>                          fpos_t _offset; \

Oops, thanks for this fix.

> Then I got these errors:
> fflush.c:38: error: dereferencing pointer to incomplete type
> fseeko.c:67: error: dereferencing pointer to incomplete type

I applied the attached fix. Thanks.

You find a new tarball at the same place:
  http://www.haible.de/bruno/gnu/testdir-stdioext.tar.gz

> File offset is wrong after fseek: 17.
> FAIL: test-fflush

That may be better in the new tarball.

> test-lseek.c:49: assertion failed
> Abort trap (core dumped)
> FAIL: test-lseek.sh

This is clearly a bug in your OS: lseek must fail when at attempt is made
to set a negative file position on a regular file. You can analyze it with
gdb, or extract a simple testcase for your libc + kernel developers.

> Skipping test: file operations failed.
> SKIP: test-freadable
> Skipping test: file operations failed.
> SKIP: test-freading
> Skipping test: file operations failed.
> SKIP: test-fwritable
> Skipping test: file operations failed.
> SKIP: test-fwriting
> testdir-stdioext compiles without that, but the test is
> skipped. Is that normal?

That's not normal. This is also an indication of things gone wrong
in your OS's stdio. You can single-step through it in gdb to see what
went wrong.

Bruno


2008-06-01  Bruno Haible  <bruno@...>

        * lib/stdio-impl.h (fp_ub): Use fp_. Needed for DragonFly BSD.
        Reported by Voroskoi Andras <voroskoi@...>.

diff --git a/lib/stdio-impl.h b/lib/stdio-impl.h
index 5288162..c803e88 100644
--- a/lib/stdio-impl.h
+++ b/lib/stdio-impl.h
@@ -60,7 +60,7 @@
     };
 #  define fp_ub ((struct __sfileext *) fp->_ext._base)->_ub
 # else                                         /* FreeBSD, DragonFly, MacOS X, Cygwin */
-#  define fp_ub fp->_ub
+#  define fp_ub fp_->_ub
 # endif
 
 # define HASUB(fp) (fp_ub._base != NULL)

[VOROSKOI Andras]

On Sun, Jun 01, 2008 at 11:22:28PM +0200, Bruno Haible wrote:
> > File offset is wrong after fseek: 17.
> > FAIL: test-fflush
>
> That may be better in the new tarball.

Yepp, works fine now.

> > test-lseek.c:49: assertion failed
> > Abort trap (core dumped)
> > FAIL: test-lseek.sh
>
> This is clearly a bug in your OS: lseek must fail when at attempt is made
> to set a negative file position on a regular file. You can analyze it with
> gdb, or extract a simple testcase for your libc + kernel developers.

I'll investigate into this.

> > Skipping test: file operations failed.
> > SKIP: test-freadable
> > Skipping test: file operations failed.
> > SKIP: test-freading
> > Skipping test: file operations failed.
> > SKIP: test-fwritable
> > Skipping test: file operations failed.
> > SKIP: test-fwriting
> > testdir-stdioext compiles without that, but the test is
> > skipped. Is that normal?
>
> That's not normal. This is also an indication of things gone wrong
> in your OS's stdio. You can single-step through it in gdb to see what
> went wrong.

These are fixed now too. So the only error left is the lseek one.

Thanks for the quick reply and fixes.

--
voroskoi

[VOROSKOI Andras]

On Sun, Jun 01, 2008 at 11:22:28PM +0200, Bruno Haible wrote:
> > test-lseek.c:49: assertion failed
> > Abort trap (core dumped)
> > FAIL: test-lseek.sh
>
> This is clearly a bug in your OS: lseek must fail when at attempt is made
> to set a negative file position on a regular file. You can analyze it with
> gdb, or extract a simple testcase for your libc + kernel developers.

Just for the record:
http://www.dragonflybsd.org/cvsweb/src/sys/kern/vfs_syscalls.c.diff?r1=1.129&r2=1.130
http://www.dragonflybsd.org/cvsweb/src/sys/kern/vfs_syscalls.c.diff?r1=1.130&r2=1.131

These two commits fixes the problem and all the tests run fine.

Thanks Bruno for your help!

--
voroskoi

[Bruno Haible]

VOROSKOI Andras wrote:
> These two commits fixes the problem and all the tests run fine.

Thanks! This means that the next version of m4 will work out-of-the-box on
DragonFly.

Also, if the DragonFly libc developers want to introduce more extended stdio
primitives, like __sreadahead, gnulib may use them. This would reduce the
need to peek inside the FILE structure from outside libc. Otherwise, the stuff
in stdio-impl.h will need updates each time the stdio internals change.

Bruno