argp: do not call _IO_fwide() if _LIBC is not defined

Message ID 20181128114839.5680-1-charles-antoine.couret@essensium.com
State New
Headers show
Series
  • argp: do not call _IO_fwide() if _LIBC is not defined
Related show

Commit Message

Couret Charles-Antoine Nov. 28, 2018, 11:48 a.m.
_IO_fwide() is defined in libio.h file. This file is included only
when _LIBC is defined.

So, in case of compilation of these files without _LIBC definition,
the compilation failed due to this unknown function.

Now this function is called when libio.h file is included.
---
 argp/argp-fmtstream.c | 4 ++++
 argp/argp-help.c      | 2 ++
 2 files changed, 6 insertions(+)

-- 
2.19.2

Comments

Joseph Myers Nov. 28, 2018, 4:51 p.m. | #1
On Wed, 28 Nov 2018, Charles-Antoine Couret wrote:

> _IO_fwide() is defined in libio.h file. This file is included only

> when _LIBC is defined.

> 

> So, in case of compilation of these files without _LIBC definition,

> the compilation failed due to this unknown function.


To confirm: is this (as it looks like) a change that is bringing in some 
conditionals from gnulib, in exactly the form in which they appear in the 
gnulib version of this code, and are there other differences between the 
two versions that would also be desirable to merge?  (The gnulib version 
is the one that is expected to be portable for use outside of glibc.  
Ideally the glibc and gnulib versions of shared code would be identical.)

-- 
Joseph S. Myers
joseph@codesourcery.com
Zack Weinberg Nov. 28, 2018, 5 p.m. | #2
On Wed, Nov 28, 2018 at 11:52 AM Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 28 Nov 2018, Charles-Antoine Couret wrote:

>

> > _IO_fwide() is defined in libio.h file. This file is included only

> > when _LIBC is defined.

> >

> > So, in case of compilation of these files without _LIBC definition,

> > the compilation failed due to this unknown function.

>

> To confirm: is this (as it looks like) a change that is bringing in some

> conditionals from gnulib, in exactly the form in which they appear in the

> gnulib version of this code, and are there other differences between the

> two versions that would also be desirable to merge?  (The gnulib version

> is the one that is expected to be portable for use outside of glibc.

> Ideally the glibc and gnulib versions of shared code would be identical.)


I'd also point out that the portable outside-libc version might be
advised to use fwide() here, the public equivalent of _IO_fwide(),
instead of just assuming the stream is narrow-oriented.

zw
Joseph Myers Nov. 28, 2018, 5:10 p.m. | #3
On Wed, 28 Nov 2018, Zack Weinberg wrote:

> I'd also point out that the portable outside-libc version might be

> advised to use fwide() here, the public equivalent of _IO_fwide(),

> instead of just assuming the stream is narrow-oriented.


Yes.  (But that's not a requirement for this patch, which could be applied 
as-is, subject to a glibc testsuite run, if indeed it matches the gnulib 
code.)

-- 
Joseph S. Myers
joseph@codesourcery.com
Couret Charles-Antoine Nov. 28, 2018, 5:25 p.m. | #4
Hi,

BTW I was not aware about differences between glibc and gnulib.

I checked the current code of gnulib and in fact they already made this 
change in the past.

See: 
http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=14a582531cef13c855c9e9638aac630c0a43ae37


And apparently the difference exists also in misc/err.c file, in glibc 
the include is not protected (and _IO_fwide() too) but in gnulib the 
same preprocessor instruction protects the header inclusion and the 
function call.

Regards,

Charles-Antoine
Joseph Myers Nov. 29, 2018, 5:58 p.m. | #5
I've now committed this patch after testing on x86_64.

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/argp/argp-fmtstream.c b/argp/argp-fmtstream.c
index e43a0c7cf1..e9e4c0e5cc 100644
--- a/argp/argp-fmtstream.c
+++ b/argp/argp-fmtstream.c
@@ -149,9 +149,11 @@  __argp_fmtstream_update (argp_fmtstream_t fs)
 	      size_t i;
 	      for (i = 0; i < pad; i++)
 		{
+#ifdef _LIBC
 		  if (_IO_fwide (fs->stream, 0) > 0)
 		    putwc_unlocked (L' ', fs->stream);
 		  else
+#endif
 		    putc_unlocked (' ', fs->stream);
 		}
 	    }
@@ -312,9 +314,11 @@  __argp_fmtstream_update (argp_fmtstream_t fs)
 	      *nl++ = ' ';
 	  else
 	    for (i = 0; i < fs->wmargin; ++i)
+#ifdef _LIBC
 	      if (_IO_fwide (fs->stream, 0) > 0)
 		putwc_unlocked (L' ', fs->stream);
 	      else
+#endif
 		putc_unlocked (' ', fs->stream);
 
 	  /* Copy the tail of the original buffer into the current buffer
diff --git a/argp/argp-help.c b/argp/argp-help.c
index 2b6b0775d6..a17260378c 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -1873,9 +1873,11 @@  __argp_failure (const struct argp_state *state, int status, int errnum,
 #endif
 	    }
 
+#ifdef _LIBC
 	  if (_IO_fwide (stream, 0) > 0)
 	    putwc_unlocked (L'\n', stream);
 	  else
+#endif
 	    putc_unlocked ('\n', stream);
 
 #if _LIBC || (HAVE_FLOCKFILE && HAVE_FUNLOCKFILE)