diff mbox

Fix open_memstream namespace (bug 18498)

Message ID alpine.DEB.2.10.1506052230060.16975@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers June 5, 2015, 10:30 p.m. UTC
open_memstream is new in the 2008 edition of POSIX.  However, the
older functions getopt, closelog and fmtmsg all bring in references to
it.  This patch fixes this in the usual way, making open_memstream
into a weak alias of __open_memstream and calling __open_memstream
from the relevant places.

Tested for x86_64 and x86 (testsuite, and that disassembly of
installed shared libraries is unchanged by the patch).  32-bit builds
produce a XPASS for conform/POSIX/unistd.h/linknamespace after this
patch (because the only cause of failure left there now is 64-bit
specific); that will disappear once the 64-bit failure is resolved and
the XFAIL removed at that time.

2015-06-05  Joseph Myers  <joseph@codesourcery.com>

	[BZ #18498]
	* libio/memstream.c (open_memstream): Rename to __open_memstream
	and define as weak alias of __open_memstream.
	* include/stdio.h (__open_memstream): Declare.  Use
	libc_hidden_proto.
	(open_memstream): Don't use libc_hidden_proto.
	* misc/syslog.c (__vsyslog_chk): Call __open_memstream instead of
	open_memstream.
	* posix/getopt.c (_getopt_internal_r): Likewise.
	* conform/Makefile (test-xfail-XPG3/stdio.h/linknamespace): Remove
	variable.
	(test-xfail-XPG4/stdio.h/linknamespace): Likewise.
	(test-xfail-UNIX98/stdio.h/linknamespace): Likewise.
	(test-xfail-XOPEN2K/unistd.h/linknamespace): Likewise.

Comments

Roland McGrath June 5, 2015, 10:37 p.m. UTC | #1
> +/* The prototype needs repeating instead of using __typeof to use
> +   __THROW in C++.  */
> +extern FILE *__open_memstream (char **, size_t *) __THROW __wur;
> +libc_hidden_proto (__open_memstream)

Does "extern __typeof (open_memstream) __open_memstream __THROW __wur;" not
work?  That would be less duplication anyway.  Regardless, the comment
reads as odd in its mention of C++ since there is C++ code in libc to be
calling this.

Substantively the change looks fine to me.
Joseph Myers June 5, 2015, 10:56 p.m. UTC | #2
On Fri, 5 Jun 2015, Roland McGrath wrote:

> > +/* The prototype needs repeating instead of using __typeof to use
> > +   __THROW in C++.  */
> > +extern FILE *__open_memstream (char **, size_t *) __THROW __wur;
> > +libc_hidden_proto (__open_memstream)
> 
> Does "extern __typeof (open_memstream) __open_memstream __THROW __wur;" not
> work?  That would be less duplication anyway.  Regardless, the comment
> reads as odd in its mention of C++ since there is C++ code in libc to be
> calling this.

The C++ code is in testcases, which produce an "expected initializer 
before 'throw'" error if you use the typeof construct (that is, C++ throw 
() only works with an actual function declarator, not when typeof is used 
to give something a function type).  Hence the comment, to explain why 
__typeof isn't used.
Roland McGrath June 6, 2015, 6:44 p.m. UTC | #3
> The C++ code is in testcases, which produce an "expected initializer 
> before 'throw'" error if you use the typeof construct (that is, C++ throw 
> () only works with an actual function declarator, not when typeof is used 
> to give something a function type).  Hence the comment, to explain why 
> __typeof isn't used.

OK.  It would not have looked so odd to me if the comment said "C++ tests".
Perhaps also some sort of XXX/TODO comment about how this should go away
when we one day stop building tests with the internal headers.


Thanks,
Roland
Roland McGrath June 6, 2015, 6:47 p.m. UTC | #4
> > The C++ code is in testcases, which produce an "expected initializer 
> > before 'throw'" error if you use the typeof construct (that is, C++ throw 
> > () only works with an actual function declarator, not when typeof is used 
> > to give something a function type).  Hence the comment, to explain why 
> > __typeof isn't used.
> 
> OK.  It would not have looked so odd to me if the comment said "C++ tests".
> Perhaps also some sort of XXX/TODO comment about how this should go away
> when we one day stop building tests with the internal headers.

Or the decl could just use typeof and be under #ifdef _LIBC, no?
Joseph Myers June 8, 2015, 10:30 a.m. UTC | #5
On Sat, 6 Jun 2015, Roland McGrath wrote:

> > > The C++ code is in testcases, which produce an "expected initializer 
> > > before 'throw'" error if you use the typeof construct (that is, C++ throw 
> > > () only works with an actual function declarator, not when typeof is used 
> > > to give something a function type).  Hence the comment, to explain why 
> > > __typeof isn't used.
> > 
> > OK.  It would not have looked so odd to me if the comment said "C++ tests".
> > Perhaps also some sort of XXX/TODO comment about how this should go away
> > when we one day stop building tests with the internal headers.
> 
> Or the decl could just use typeof and be under #ifdef _LIBC, no?

I've made the comment say "C++ tests".  This matches what we do in 
wchar.h.  As far as I know we still define _LIBC when building tests, as 
well as using internal headers for them.
diff mbox

Patch

diff --git a/conform/Makefile b/conform/Makefile
index 662ea96..943a196 100644
--- a/conform/Makefile
+++ b/conform/Makefile
@@ -347,14 +347,12 @@  test-xfail-XOPEN2K8/ndbm.h/linknamespace = yes
 # Unsorted expected failures.
 test-xfail-XPG3/glob.h/linknamespace = yes
 test-xfail-XPG3/regex.h/linknamespace = yes
-test-xfail-XPG3/stdio.h/linknamespace = yes
 test-xfail-XPG3/unistd.h/linknamespace = yes
 test-xfail-XPG3/wordexp.h/linknamespace = yes
 test-xfail-XPG4/fmtmsg.h/linknamespace = yes
 test-xfail-XPG4/glob.h/linknamespace = yes
 test-xfail-XPG4/netdb.h/linknamespace = yes
 test-xfail-XPG4/regex.h/linknamespace = yes
-test-xfail-XPG4/stdio.h/linknamespace = yes
 test-xfail-XPG4/stdlib.h/linknamespace = yes
 test-xfail-XPG4/syslog.h/linknamespace = yes
 test-xfail-XPG4/unistd.h/linknamespace = yes
@@ -367,7 +365,6 @@  test-xfail-POSIX/unistd.h/linknamespace = yes
 test-xfail-UNIX98/fmtmsg.h/linknamespace = yes
 test-xfail-UNIX98/mqueue.h/linknamespace = yes
 test-xfail-UNIX98/netdb.h/linknamespace = yes
-test-xfail-UNIX98/stdio.h/linknamespace = yes
 test-xfail-UNIX98/stdlib.h/linknamespace = yes
 test-xfail-UNIX98/syslog.h/linknamespace = yes
 test-xfail-UNIX98/unistd.h/linknamespace = yes
@@ -376,7 +373,6 @@  test-xfail-XOPEN2K/fmtmsg.h/linknamespace = yes
 test-xfail-XOPEN2K/netdb.h/linknamespace = yes
 test-xfail-XOPEN2K/stdlib.h/linknamespace = yes
 test-xfail-XOPEN2K/syslog.h/linknamespace = yes
-test-xfail-XOPEN2K/unistd.h/linknamespace = yes
 test-xfail-POSIX2008/grp.h/linknamespace = yes
 test-xfail-POSIX2008/netdb.h/linknamespace = yes
 test-xfail-POSIX2008/semaphore.h/linknamespace = yes
diff --git a/include/stdio.h b/include/stdio.h
index 043b2b5..78bd3c4 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -160,7 +160,10 @@  extern __typeof (fgets_unlocked) __fgets_unlocked;
 libc_hidden_proto (__fgets_unlocked)
 libc_hidden_proto (fputs_unlocked)
 libc_hidden_proto (fmemopen)
-libc_hidden_proto (open_memstream)
+/* The prototype needs repeating instead of using __typeof to use
+   __THROW in C++.  */
+extern FILE *__open_memstream (char **, size_t *) __THROW __wur;
+libc_hidden_proto (__open_memstream)
 libc_hidden_proto (__libc_fatal)
 libc_hidden_proto (__vsprintf_chk)
 libc_hidden_proto (__vsnprintf_chk)
diff --git a/libio/memstream.c b/libio/memstream.c
index 81bfe6d..84742d1 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -61,7 +61,7 @@  static const struct _IO_jump_t _IO_mem_jumps =
    necessary.  *BUFLOC and *SIZELOC are updated with the buffer's location
    and the number of characters written on fflush or fclose.  */
 _IO_FILE *
-open_memstream (bufloc, sizeloc)
+__open_memstream (bufloc, sizeloc)
      char **bufloc;
      _IO_size_t *sizeloc;
 {
@@ -100,7 +100,8 @@  open_memstream (bufloc, sizeloc)
 
   return (_IO_FILE *) &new_f->fp._sf._sbf;
 }
-libc_hidden_def (open_memstream)
+libc_hidden_def (__open_memstream)
+weak_alias (__open_memstream, open_memstream)
 
 
 static int
diff --git a/misc/syslog.c b/misc/syslog.c
index 70daa9e..23a4f7b 100644
--- a/misc/syslog.c
+++ b/misc/syslog.c
@@ -164,7 +164,7 @@  __vsyslog_chk(int pri, int flag, const char *fmt, va_list ap)
 		pri |= LogFacility;
 
 	/* Build the message in a memory-buffer stream.  */
-	f = open_memstream (&buf, &bufsize);
+	f = __open_memstream (&buf, &bufsize);
 	if (f == NULL)
 	  {
 	    /* We cannot get a stream.  There is not much we can do but
diff --git a/posix/getopt.c b/posix/getopt.c
index 841392e..d30530f 100644
--- a/posix/getopt.c
+++ b/posix/getopt.c
@@ -585,7 +585,7 @@  _getopt_internal_r (int argc, char *const *argv, const char *optstring,
 	      char *buf = NULL;
 	      size_t buflen = 0;
 
-	      FILE *fp = open_memstream (&buf, &buflen);
+	      FILE *fp = __open_memstream (&buf, &buflen);
 	      if (fp != NULL)
 		{
 		  fprintf (fp,