Re: Unused parameter in binary-io.h?

2018-03-05 Thread Reuben Thomas
On 5 March 2018 at 23:32, Paul Eggert  wrote:
> On 03/05/2018 01:50 PM, Reuben Thomas wrote:
>>
>> Is there some reason that the unused parameter is not suitably
>> decorated for gcc?
>
>
> I expect it is because nobody has gotten around to it. I installed the
> attached.

Thanks very much! I wasn't sure; often I trip over subtleties in
gnulib (understandably, given what it does).

-- 
https://rrt.sc3d.org



Re: Unused parameter in binary-io.h?

2018-03-05 Thread Paul Eggert

On 03/05/2018 01:50 PM, Reuben Thomas wrote:

Is there some reason that the unused parameter is not suitably
decorated for gcc?


I expect it is because nobody has gotten around to it. I installed the 
attached.


>From 746fe7d6c56f87e9f51679acf2f7b8464520baac Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Mon, 5 Mar 2018 15:30:56 -0800
Subject: [PATCH] binary-io: pacify gcc -Wunused-parameter

Problem reported by Reuben Thomas in:
https://lists.gnu.org/r/bug-gnulib/2018-03/msg5.html
* lib/binary-io.h (__gl_setmode, __gl_setmode_check):
Use _GL_UNUSED where appropriate.
---
 ChangeLog   | 6 ++
 lib/binary-io.h | 6 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index beb835670..8f372dfb9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2018-03-05  Paul Eggert  
 
+	binary-io: pacify gcc -Wunused-parameter
+	Problem reported by Reuben Thomas in:
+	https://lists.gnu.org/r/bug-gnulib/2018-03/msg5.html
+	* lib/binary-io.h (__gl_setmode, __gl_setmode_check):
+	Use _GL_UNUSED where appropriate.
+
 	fflush: adjust to glibc 2.28 libio.h removal
 	Problem reported by Daniel P. Berrangé in:
 	https://lists.gnu.org/r/bug-gnulib/2018-03/msg0.html
diff --git a/lib/binary-io.h b/lib/binary-io.h
index cce1301d5..1f21fc051 100644
--- a/lib/binary-io.h
+++ b/lib/binary-io.h
@@ -47,10 +47,8 @@ _GL_INLINE_HEADER_BEGIN
   /* Use a function rather than a macro, to avoid gcc warnings
  "warning: statement with no effect".  */
 BINARY_IO_INLINE int
-__gl_setmode (int fd, int mode)
+__gl_setmode (int fd _GL_UNUSED, int mode _GL_UNUSED)
 {
-  (void) fd;
-  (void) mode;
   return O_BINARY;
 }
 #endif
@@ -59,7 +57,7 @@ __gl_setmode (int fd, int mode)
 extern int __gl_setmode_check (int);
 #else
 BINARY_IO_INLINE int
-__gl_setmode_check (int fd) { return 0; }
+__gl_setmode_check (int fd _GL_UNUSED) { return 0; }
 #endif
 
 /* Set FD's mode to MODE, which should be either O_TEXT or O_BINARY.
-- 
2.14.3



Unused parameter in binary-io.h?

2018-03-05 Thread Reuben Thomas
I get the following warnings when compiling with that header on a GNU system:

../lib/binary-io.h: In function ‘__gl_setmode_check’:
../lib/binary-io.h:62:25: warning: unused parameter ‘fd’ [-Wunused-parameter]
 __gl_setmode_check (int fd) { return 0; }

Is there some reason that the unused parameter is not suitably
decorated for gcc?

-- 
https://rrt.sc3d.org



Re: fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)

2018-03-05 Thread Paul Eggert

On 03/05/2018 11:20 AM, Daniel P. Berrangé wrote:

Yes, this worked on rawhide when I tested with libvirt.


Thanks, I installed it into Gnulib since it was breaking builds. Bruno's 
the expert here, and perhaps he can come up with something better.





Re: fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)

2018-03-05 Thread Daniel P . Berrangé
On Mon, Mar 05, 2018 at 11:00:40AM -0800, Paul Eggert wrote:
> On 03/05/2018 04:19 AM, Daniel P. Berrangé wrote:
> > I'm curious why we're trying to replace fseeko impl at all, since I
> > would expect the modern glibc impl to be suitable to use as-is.
> 
> It's to work around glibc bug#12799, a longstanding bug with ungetc and
> fflush. See the bug-gnulib thread that starts here:
> 
> https://lists.gnu.org/r/bug-gnulib/2009-01/msg00067.html
> 
> Any chance of getting that glibc bug fixed in glibc 2.28? That should avoid
> the problem with Gnulib, and would be a good thing in its own right. In the
> meantime, the attached Gnulib patch is intended to work around the bug from
> the Gnulib side; does it work for you? (I haven't tried it on rawhide.)

Yes, this worked on rawhide when I tested with libvirt. Though note that
libvirt doesn't pull in all of the files you've changed here, so my test
hasn't validated everything.


> From 1831628c0630ae96a43586b2a25ca51cbdba3e53 Mon Sep 17 00:00:00 2001
> From: Paul Eggert 
> Date: Mon, 5 Mar 2018 10:56:29 -0800
> Subject: [PATCH] fflush: adjust to glibc 2.28 libio.h removal
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Problem reported by Daniel P. Berrangé in:
> https://lists.gnu.org/r/bug-gnulib/2018-03/msg0.html
> * lib/fbufmode.c (fbufmode):
> * lib/fflush.c (clear_ungetc_buffer_preserving_position)
> (disable_seek_optimization, rpl_fflush):
> * lib/fpending.c (__fpending):
> * lib/fpurge.c (fpurge):
> * lib/freadable.c (freadable):
> * lib/freadahead.c (freadahead):
> * lib/freading.c (freading):
> * lib/freadptr.c (freadptr):
> * lib/freadseek.c (freadptrinc):
> * lib/fseeko.c (fseeko):
> * lib/fseterr.c (fseterr):
> * lib/fwritable.c (fwritable):
> * lib/fwriting.c (fwriting):
> Check _IO_EOF_SEEN instead of _IO_ftrylockfile.
> * lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]:
> Define if not already defined.
> ---
>  ChangeLog| 23 +++
>  lib/fbufmode.c   |  2 +-
>  lib/fflush.c |  6 +++---
>  lib/fpending.c   |  2 +-
>  lib/fpurge.c |  2 +-
>  lib/freadable.c  |  2 +-
>  lib/freadahead.c |  2 +-
>  lib/freading.c   |  2 +-
>  lib/freadptr.c   |  2 +-
>  lib/freadseek.c  |  2 +-
>  lib/fseeko.c |  4 ++--
>  lib/fseterr.c|  2 +-
>  lib/fwritable.c  |  2 +-
>  lib/fwriting.c   |  2 +-
>  lib/stdio-impl.h |  6 ++
>  15 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 667f91663..beb835670 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,26 @@
> +2018-03-05  Paul Eggert  
> +
> + fflush: adjust to glibc 2.28 libio.h removal
> + Problem reported by Daniel P. Berrangé in:
> + https://lists.gnu.org/r/bug-gnulib/2018-03/msg0.html
> + * lib/fbufmode.c (fbufmode):
> + * lib/fflush.c (clear_ungetc_buffer_preserving_position)
> + (disable_seek_optimization, rpl_fflush):
> + * lib/fpending.c (__fpending):
> + * lib/fpurge.c (fpurge):
> + * lib/freadable.c (freadable):
> + * lib/freadahead.c (freadahead):
> + * lib/freading.c (freading):
> + * lib/freadptr.c (freadptr):
> + * lib/freadseek.c (freadptrinc):
> + * lib/fseeko.c (fseeko):
> + * lib/fseterr.c (fseterr):
> + * lib/fwritable.c (fwritable):
> + * lib/fwriting.c (fwriting):
> + Check _IO_EOF_SEEN instead of _IO_ftrylockfile.
> + * lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]:
> + Define if not already defined.
> +
>  2018-02-27  Paul Eggert  
>  
>   environ: fix link error on 32-bit Cygwin
> diff --git a/lib/fbufmode.c b/lib/fbufmode.c
> index 73aa5fc07..f711fdfb7 100644
> --- a/lib/fbufmode.c
> +++ b/lib/fbufmode.c
> @@ -31,7 +31,7 @@ fbufmode (FILE *fp)
>/* Most systems provide FILE as a struct and the necessary bitmask in
>   , because they need it for implementing getc() and putc() as
>   fast macros.  */
> -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
> Haiku, Linux libc5 */
> +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
> Linux libc5 */
>  # if HAVE___FLBF/* glibc >= 2.2 */
>if (__flbf (fp))
>  return _IOLBF;
> diff --git a/lib/fflush.c b/lib/fflush.c
> index 983ade0ff..a6edfa105 100644
> --- a/lib/fflush.c
> +++ b/lib/fflush.c
> @@ -33,7 +33,7 @@
>  #undef fflush
>  
>  
> -#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
> Haiku, Linux libc5 */
> +#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
> Linux libc5 */
>  
>  /* Clear the stream's ungetc buffer, preserving the value of ftello (fp).  */
>  static void
> @@ -72,7 +72,7 @@ clear_ungetc_buffer (FILE *fp)
>  
>  #endif
>  
> -#if ! (defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
> Haiku, Linux libc5 */)
> +#if ! (defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, 

Re: fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)

2018-03-05 Thread Paul Eggert

On 03/05/2018 04:19 AM, Daniel P. Berrangé wrote:

I'm curious why we're trying to replace fseeko impl at all, since I
would expect the modern glibc impl to be suitable to use as-is.


It's to work around glibc bug#12799, a longstanding bug with ungetc and 
fflush. See the bug-gnulib thread that starts here:


https://lists.gnu.org/r/bug-gnulib/2009-01/msg00067.html

Any chance of getting that glibc bug fixed in glibc 2.28? That should 
avoid the problem with Gnulib, and would be a good thing in its own 
right. In the meantime, the attached Gnulib patch is intended to work 
around the bug from the Gnulib side; does it work for you? (I haven't 
tried it on rawhide.)


From 1831628c0630ae96a43586b2a25ca51cbdba3e53 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Mon, 5 Mar 2018 10:56:29 -0800
Subject: [PATCH] fflush: adjust to glibc 2.28 libio.h removal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem reported by Daniel P. Berrangé in:
https://lists.gnu.org/r/bug-gnulib/2018-03/msg0.html
* lib/fbufmode.c (fbufmode):
* lib/fflush.c (clear_ungetc_buffer_preserving_position)
(disable_seek_optimization, rpl_fflush):
* lib/fpending.c (__fpending):
* lib/fpurge.c (fpurge):
* lib/freadable.c (freadable):
* lib/freadahead.c (freadahead):
* lib/freading.c (freading):
* lib/freadptr.c (freadptr):
* lib/freadseek.c (freadptrinc):
* lib/fseeko.c (fseeko):
* lib/fseterr.c (fseterr):
* lib/fwritable.c (fwritable):
* lib/fwriting.c (fwriting):
Check _IO_EOF_SEEN instead of _IO_ftrylockfile.
* lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]:
Define if not already defined.
---
 ChangeLog| 23 +++
 lib/fbufmode.c   |  2 +-
 lib/fflush.c |  6 +++---
 lib/fpending.c   |  2 +-
 lib/fpurge.c |  2 +-
 lib/freadable.c  |  2 +-
 lib/freadahead.c |  2 +-
 lib/freading.c   |  2 +-
 lib/freadptr.c   |  2 +-
 lib/freadseek.c  |  2 +-
 lib/fseeko.c |  4 ++--
 lib/fseterr.c|  2 +-
 lib/fwritable.c  |  2 +-
 lib/fwriting.c   |  2 +-
 lib/stdio-impl.h |  6 ++
 15 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 667f91663..beb835670 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2018-03-05  Paul Eggert  
+
+   fflush: adjust to glibc 2.28 libio.h removal
+   Problem reported by Daniel P. Berrangé in:
+   https://lists.gnu.org/r/bug-gnulib/2018-03/msg0.html
+   * lib/fbufmode.c (fbufmode):
+   * lib/fflush.c (clear_ungetc_buffer_preserving_position)
+   (disable_seek_optimization, rpl_fflush):
+   * lib/fpending.c (__fpending):
+   * lib/fpurge.c (fpurge):
+   * lib/freadable.c (freadable):
+   * lib/freadahead.c (freadahead):
+   * lib/freading.c (freading):
+   * lib/freadptr.c (freadptr):
+   * lib/freadseek.c (freadptrinc):
+   * lib/fseeko.c (fseeko):
+   * lib/fseterr.c (fseterr):
+   * lib/fwritable.c (fwritable):
+   * lib/fwriting.c (fwriting):
+   Check _IO_EOF_SEEN instead of _IO_ftrylockfile.
+   * lib/stdio-impl.h (_IO_IN_BACKUP) [_IO_EOF_SEEN]:
+   Define if not already defined.
+
 2018-02-27  Paul Eggert  
 
environ: fix link error on 32-bit Cygwin
diff --git a/lib/fbufmode.c b/lib/fbufmode.c
index 73aa5fc07..f711fdfb7 100644
--- a/lib/fbufmode.c
+++ b/lib/fbufmode.c
@@ -31,7 +31,7 @@ fbufmode (FILE *fp)
   /* Most systems provide FILE as a struct and the necessary bitmask in
  , because they need it for implementing getc() and putc() as
  fast macros.  */
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */
+#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */
 # if HAVE___FLBF/* glibc >= 2.2 */
   if (__flbf (fp))
 return _IOLBF;
diff --git a/lib/fflush.c b/lib/fflush.c
index 983ade0ff..a6edfa105 100644
--- a/lib/fflush.c
+++ b/lib/fflush.c
@@ -33,7 +33,7 @@
 #undef fflush
 
 
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */
+#if defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */
 
 /* Clear the stream's ungetc buffer, preserving the value of ftello (fp).  */
 static void
@@ -72,7 +72,7 @@ clear_ungetc_buffer (FILE *fp)
 
 #endif
 
-#if ! (defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
Haiku, Linux libc5 */)
+#if ! (defined _IO_EOF_SEEN || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */)
 
 # if (defined __sferror || defined __DragonFly__ || defined __ANDROID__) && 
defined __SNPT
 /* FreeBSD, NetBSD, OpenBSD, DragonFly, Mac OS X, Cygwin, Minix 3, Android */
@@ -148,7 +148,7 @@ rpl_fflush (FILE *stream)
   if (stream == NULL || ! freading (stream))
 return fflush (stream);
 
-#if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, Haiku, 
Linux libc5 */
+#if defined 

Re: [libvirt] fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)

2018-03-05 Thread Daniel P . Berrangé
On Mon, Mar 05, 2018 at 12:19:24PM +, Daniel P. Berrangé wrote:
> Fedora rawhide has just upgraded to the latest glibc git master snapshot,
> of what will become the 2.28 release, and this has exposed bugs in gnulib's
> fseeko.c implementation (and probably more macros) besides.
> 
> The issue starts are line 50:
> 
>   http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fseeko.c#n50
> 
>   #if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
> Haiku, Linux libc5 */
> 
> Historically the _IO_ftrylockfile symbol has been defined unconditionally
> in glibc, by /usr/include/libio.h, which is pulled in unconditionally from
> /usr/include/stdio.h
> 
> The libio.h header was deprecated in 2.27 release and is removed in git
> master for future 2.28. Thus the _IO_ftrylockfile symbol has gone away.
> 
>   https://sourceware.org/ml/libc-announce/2018/msg0.html
> 
>  "The nonstandard header files  and <_G_config.h> are deprecated
>   and will be removed in a future release.  Software that is still using
>   either header should be updated to use standard  interfaces
>   instead.
> 
>   libio.h was originally the header for a set of supported GNU extensions,
>   but they have not been maintained as such in many years, they are now
>   standing in the way of improvements to stdio, and we don't think there are
>   any remaining external users.  _G_config.h was never intended for public
>   use, but predates the bits convention."
> 
> 
> We then fail the __GNU_LIBRARY__ test too, because modern glibc defines
> that to a value of '6', not '1'.
> 
> For added fun __GNU_LIBRARY__ is considered deprecated too, with
> recommendation to use other symbols like __GLIBC__ and __GLIBC_MINOR__
> 
> 
>   /* This macro indicates that the installed library is the GNU C Library.
>  For historic reasons the value now is 6 and this will stay from now
>  on.  The use of this variable is deprecated.  Use __GLIBC__ and
>  __GLIBC_MINOR__ now (see below) when you want to test for a specific
>  GNU C library version and use the values in  to get
>  the sonames of the shared libraries.  *
>   #define __GNU_LIBRARY__ 6
> 
> I hit failure on fseeko.c, but many other files in gnulib test on
> _IO_ftrylockfile so I presume they are all broken.
> 
> I'm curious why we're trying to replace fseeko impl at all, since I
> would expect the modern glibc impl to be suitable to use as-is.

Changing the test for  '__GNU_LIBRARY__ == 1' to just "__GNU_LIBRARY__"
does appear to work at first, but fflush.c then fails

fflush.c: In function 'clear_ungetc_buffer_preserving_position':
fflush.c:42:20: error: '_IO_IN_BACKUP' undeclared (first use in this function)
   if (fp->_flags & _IO_IN_BACKUP)
^

This constant was also in libio.h and so no longer defined. Re-defining
it in gnulib feels dangerous because glibc is free to change its value
at any time now this is no longer a public API symbol.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



fseeko broken by Fedora rawhide glibc / git master (ie future 2.28)

2018-03-05 Thread Daniel P . Berrangé
Fedora rawhide has just upgraded to the latest glibc git master snapshot,
of what will become the 2.28 release, and this has exposed bugs in gnulib's
fseeko.c implementation (and probably more macros) besides.

The issue starts are line 50:

  http://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/fseeko.c#n50

  #if defined _IO_ftrylockfile || __GNU_LIBRARY__ == 1 /* GNU libc, BeOS, 
Haiku, Linux libc5 */

Historically the _IO_ftrylockfile symbol has been defined unconditionally
in glibc, by /usr/include/libio.h, which is pulled in unconditionally from
/usr/include/stdio.h

The libio.h header was deprecated in 2.27 release and is removed in git
master for future 2.28. Thus the _IO_ftrylockfile symbol has gone away.

  https://sourceware.org/ml/libc-announce/2018/msg0.html

 "The nonstandard header files  and <_G_config.h> are deprecated
  and will be removed in a future release.  Software that is still using
  either header should be updated to use standard  interfaces
  instead.

  libio.h was originally the header for a set of supported GNU extensions,
  but they have not been maintained as such in many years, they are now
  standing in the way of improvements to stdio, and we don't think there are
  any remaining external users.  _G_config.h was never intended for public
  use, but predates the bits convention."


We then fail the __GNU_LIBRARY__ test too, because modern glibc defines
that to a value of '6', not '1'.

For added fun __GNU_LIBRARY__ is considered deprecated too, with
recommendation to use other symbols like __GLIBC__ and __GLIBC_MINOR__


  /* This macro indicates that the installed library is the GNU C Library.
 For historic reasons the value now is 6 and this will stay from now
 on.  The use of this variable is deprecated.  Use __GLIBC__ and
 __GLIBC_MINOR__ now (see below) when you want to test for a specific
 GNU C library version and use the values in  to get
 the sonames of the shared libraries.  *
  #define __GNU_LIBRARY__ 6

I hit failure on fseeko.c, but many other files in gnulib test on
_IO_ftrylockfile so I presume they are all broken.

I'm curious why we're trying to replace fseeko impl at all, since I
would expect the modern glibc impl to be suitable to use as-is.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|