Message ID | 20140709103302.GR609@spoyarek.pnq.redhat.com |
---|---|
State | New |
Headers | show |
On 9 July 2014 11:33, Siddhesh Poyarekar <siddhesh@redhat.com> wrote: > hi, > > The following patch should sync up the glibc version of error.c with > gnulib. There are a few changes that are pending inclusion into > gnulib; I have posted them to bug-gnulib but the email is probably > held in moderation. > > Summary of changes: > > - Use of !_LIBC instead of HAVE_CONFIG_H > - Code changes in [!_LIBC] that don't affect us > - Minor formatting changes > - Use __builtin_expect in shared code > - Define some macros in [_LIBC] that are used in gnulib but never > defined in glibc (change proposed in gnulib) > - Flip macro check for STRERROR_R_CHAR_P so that it does not throw a > warning (change proposed in gnulib) AFAICT STRERROR_R_CHAR_P is not defined in gnulib any more so it should probably be removed altogether. > The only change in generated code on x86_64 is due to change in line > numbers for error_at_line. OK to commit? > > Siddhesh > > Sync up with gnulib. > * misc/error.c: Use !_LIBC instead of HAVE_CONFIG_H. > [!_LIBC && ENABLE_NLS]: Include gettext.h. > [_LIBC]: Define USE_UNLOCKED_IO, _GL_ATTRIBUTE_FORMAT_PRINTF > and _GL_ARG_NONNULL. > [USE_UNLOCKED_IO]: Include unlocked-io.h. > [!_LIBC]: Include code for Windows and Cygwin. > [!_LIBC && !HAVE_DECL_STRERROR_R && !STRERROR_R_CHAR_P]: > Include prototype for int strerror_r. > [!_LIBC] (is_open): New function. > (flush_stdout): New function. > (print_errno_message): Use it. > (error): Likewise. > (error_at_line): Likewise. > (error_tail) Add function attribute macros. Use > __builtin_expect. > > diff --git a/misc/error.c b/misc/error.c > index 63bd309..ee0cf86 100644 > --- a/misc/error.c > +++ b/misc/error.c > @@ -18,24 +18,36 @@ > > /* Written by David MacKenzie <djm@gnu.ai.mit.edu>. */ > > -#ifdef HAVE_CONFIG_H > +#if !_LIBC > # include <config.h> > #endif > > +#include "error.h" > + > #include <stdarg.h> > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > > +#if !_LIBC && ENABLE_NLS > +# include "gettext.h" > +# define _(msgid) gettext (msgid) > +#endif > + > #ifdef _LIBC > # include <libintl.h> > # include <stdbool.h> > # include <stdint.h> > # include <wchar.h> > # define mbsrtowcs __mbsrtowcs > +# define USE_UNLOCKED_IO 0 > +# define _GL_ATTRIBUTE_FORMAT_PRINTF(a, b) > +# define _GL_ARG_NONNULL(a) > #endif > > -#include "error.h" > +#if USE_UNLOCKED_IO > +# include "unlocked-io.h" > +#endif > > #ifndef _ > # define _(String) String > @@ -46,7 +58,7 @@ > function without parameters instead. */ > void (*error_print_progname) (void); > > -/* This variable is incremented each time `error' is called. */ > +/* This variable is incremented each time 'error' is called. */ > unsigned int error_message_count; > > #ifdef _LIBC > @@ -57,7 +69,7 @@ unsigned int error_message_count; > # include <limits.h> > # include <libio/libioP.h> > > -/* In GNU libc we want do not want to use the common name `error' directly. > +/* In GNU libc we want do not want to use the common name 'error' directly. > Instead make it a weak alias. */ > extern void __error (int status, int errnum, const char *message, ...) > __attribute__ ((__format__ (__printf__, 3, 4))); > @@ -77,11 +89,29 @@ extern void __error_at_line (int status, int errnum, const char *file_name, > > #else /* not _LIBC */ > > -# if !HAVE_DECL_STRERROR_R && STRERROR_R_CHAR_P > +# include <fcntl.h> > +# include <unistd.h> > + > +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ > +/* Get declarations of the native Windows API functions. */ > +# define WIN32_LEAN_AND_MEAN > +# include <windows.h> > +/* Get _get_osfhandle. */ > +# include "msvc-nothrow.h" > +# endif > + > +/* The gnulib override of fcntl is not needed in this file. */ > +# undef fcntl > + > +# if !HAVE_DECL_STRERROR_R > # ifndef HAVE_DECL_STRERROR_R > "this configure-time declaration test was not run" > # endif > +# if STRERROR_R_CHAR_P > char *strerror_r (); > +# else > +int strerror_r (); > +# endif > # endif > > /* The calling program should define program_name and set it to the > @@ -93,6 +123,51 @@ extern char *program_name; > # endif /* HAVE_STRERROR_R || defined strerror_r */ > #endif /* not _LIBC */ > > +#if !_LIBC > +/* Return non-zero if FD is open. */ > +static int > +is_open (int fd) > +{ > +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ > + /* On native Windows: The initial state of unassigned standard file > + descriptors is that they are open but point to an INVALID_HANDLE_VALUE. > + There is no fcntl, and the gnulib replacement fcntl does not support > + F_GETFL. */ > + return (HANDLE) _get_osfhandle (fd) != INVALID_HANDLE_VALUE; > +# else > +# ifndef F_GETFL > +# error Please port fcntl to your platform > +# endif > + return 0 <= fcntl (fd, F_GETFL); > +# endif > +} > +#endif > + > +static void > +flush_stdout (void) > +{ > +#if !_LIBC > + int stdout_fd; > + > +# if GNULIB_FREOPEN_SAFER > + /* Use of gnulib's freopen-safer module normally ensures that > + fileno (stdout) == 1 > + whenever stdout is open. */ > + stdout_fd = STDOUT_FILENO; > +# else > + /* POSIX states that fileno (stdout) after fclose is unspecified. But in > + practice it is not a problem, because stdout is statically allocated and > + the fd of a FILE stream is stored as a field in its allocated memory. */ > + stdout_fd = fileno (stdout); > +# endif > + /* POSIX states that fflush (stdout) after fclose is unspecified; it > + is safe in glibc, but not on all other platforms. fflush (NULL) > + is always defined, but too draconian. */ > + if (0 <= stdout_fd && is_open (stdout_fd)) > +#endif > + fflush (stdout); > +} > + > static void > print_errno_message (int errnum) > { > @@ -100,7 +175,7 @@ print_errno_message (int errnum) > > #if defined HAVE_STRERROR_R || _LIBC > char errbuf[1024]; > -# if STRERROR_R_CHAR_P || _LIBC > +# if _LIBC || STRERROR_R_CHAR_P > s = __strerror_r (errnum, errbuf, sizeof errbuf); > # else > if (__strerror_r (errnum, errbuf, sizeof errbuf) == 0) > @@ -124,7 +199,7 @@ print_errno_message (int errnum) > #endif > } > > -static void > +static void _GL_ATTRIBUTE_FORMAT_PRINTF (3, 0) _GL_ARG_NONNULL ((3)) > error_tail (int status, int errnum, const char *message, va_list args) > { > #if _LIBC > @@ -165,7 +240,7 @@ error_tail (int status, int errnum, const char *message, va_list args) > if (res != len) > break; > > - if (__glibc_unlikely (len >= SIZE_MAX / sizeof (wchar_t) / 2)) > + if (__builtin_expect (len >= SIZE_MAX / sizeof (wchar_t) / 2, 0)) > { > /* This really should not happen if everything is fine. */ > res = (size_t) -1; > @@ -227,7 +302,7 @@ error (int status, int errnum, const char *message, ...) > 0); > #endif > > - fflush (stdout); > + flush_stdout (); > #ifdef _LIBC > _IO_flockfile (stderr); > #endif > @@ -273,6 +348,7 @@ error_at_line (int status, int errnum, const char *file_name, > || (old_file_name != NULL > && file_name != NULL > && strcmp (old_file_name, file_name) == 0))) > + > /* Simply return and print nothing. */ > return; > > @@ -288,7 +364,7 @@ error_at_line (int status, int errnum, const char *file_name, > 0); > #endif > > - fflush (stdout); > + flush_stdout (); > #ifdef _LIBC > _IO_flockfile (stderr); > #endif
On Wed, Jul 09, 2014 at 11:41:09AM +0100, Will Newton wrote: > On 9 July 2014 11:33, Siddhesh Poyarekar <siddhesh@redhat.com> wrote: > > hi, > > > > The following patch should sync up the glibc version of error.c with > > gnulib. There are a few changes that are pending inclusion into > > gnulib; I have posted them to bug-gnulib but the email is probably > > held in moderation. > > > > Summary of changes: > > > > - Use of !_LIBC instead of HAVE_CONFIG_H > > - Code changes in [!_LIBC] that don't affect us > > - Minor formatting changes > > - Use __builtin_expect in shared code > > - Define some macros in [_LIBC] that are used in gnulib but never > > defined in glibc (change proposed in gnulib) > > - Flip macro check for STRERROR_R_CHAR_P so that it does not throw a > > warning (change proposed in gnulib) > > AFAICT STRERROR_R_CHAR_P is not defined in gnulib any more so it > should probably be removed altogether. Oh yeah, thanks for pointing that out. I'll update my patches. Siddhesh
That all seems fine if it gets us to verbatim synch with gnulib.
diff --git a/misc/error.c b/misc/error.c index 63bd309..ee0cf86 100644 --- a/misc/error.c +++ b/misc/error.c @@ -18,24 +18,36 @@ /* Written by David MacKenzie <djm@gnu.ai.mit.edu>. */ -#ifdef HAVE_CONFIG_H +#if !_LIBC # include <config.h> #endif +#include "error.h" + #include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <string.h> +#if !_LIBC && ENABLE_NLS +# include "gettext.h" +# define _(msgid) gettext (msgid) +#endif + #ifdef _LIBC # include <libintl.h> # include <stdbool.h> # include <stdint.h> # include <wchar.h> # define mbsrtowcs __mbsrtowcs +# define USE_UNLOCKED_IO 0 +# define _GL_ATTRIBUTE_FORMAT_PRINTF(a, b) +# define _GL_ARG_NONNULL(a) #endif -#include "error.h" +#if USE_UNLOCKED_IO +# include "unlocked-io.h" +#endif #ifndef _ # define _(String) String @@ -46,7 +58,7 @@ function without parameters instead. */ void (*error_print_progname) (void); -/* This variable is incremented each time `error' is called. */ +/* This variable is incremented each time 'error' is called. */ unsigned int error_message_count; #ifdef _LIBC @@ -57,7 +69,7 @@ unsigned int error_message_count; # include <limits.h> # include <libio/libioP.h> -/* In GNU libc we want do not want to use the common name `error' directly. +/* In GNU libc we want do not want to use the common name 'error' directly. Instead make it a weak alias. */ extern void __error (int status, int errnum, const char *message, ...) __attribute__ ((__format__ (__printf__, 3, 4))); @@ -77,11 +89,29 @@ extern void __error_at_line (int status, int errnum, const char *file_name, #else /* not _LIBC */ -# if !HAVE_DECL_STRERROR_R && STRERROR_R_CHAR_P +# include <fcntl.h> +# include <unistd.h> + +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ +/* Get declarations of the native Windows API functions. */ +# define WIN32_LEAN_AND_MEAN +# include <windows.h> +/* Get _get_osfhandle. */ +# include "msvc-nothrow.h" +# endif + +/* The gnulib override of fcntl is not needed in this file. */ +# undef fcntl + +# if !HAVE_DECL_STRERROR_R # ifndef HAVE_DECL_STRERROR_R "this configure-time declaration test was not run" # endif +# if STRERROR_R_CHAR_P char *strerror_r (); +# else +int strerror_r (); +# endif # endif /* The calling program should define program_name and set it to the @@ -93,6 +123,51 @@ extern char *program_name; # endif /* HAVE_STRERROR_R || defined strerror_r */ #endif /* not _LIBC */ +#if !_LIBC +/* Return non-zero if FD is open. */ +static int +is_open (int fd) +{ +# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ + /* On native Windows: The initial state of unassigned standard file + descriptors is that they are open but point to an INVALID_HANDLE_VALUE. + There is no fcntl, and the gnulib replacement fcntl does not support + F_GETFL. */ + return (HANDLE) _get_osfhandle (fd) != INVALID_HANDLE_VALUE; +# else +# ifndef F_GETFL +# error Please port fcntl to your platform +# endif + return 0 <= fcntl (fd, F_GETFL); +# endif +} +#endif + +static void +flush_stdout (void) +{ +#if !_LIBC + int stdout_fd; + +# if GNULIB_FREOPEN_SAFER + /* Use of gnulib's freopen-safer module normally ensures that + fileno (stdout) == 1 + whenever stdout is open. */ + stdout_fd = STDOUT_FILENO; +# else + /* POSIX states that fileno (stdout) after fclose is unspecified. But in + practice it is not a problem, because stdout is statically allocated and + the fd of a FILE stream is stored as a field in its allocated memory. */ + stdout_fd = fileno (stdout); +# endif + /* POSIX states that fflush (stdout) after fclose is unspecified; it + is safe in glibc, but not on all other platforms. fflush (NULL) + is always defined, but too draconian. */ + if (0 <= stdout_fd && is_open (stdout_fd)) +#endif + fflush (stdout); +} + static void print_errno_message (int errnum) { @@ -100,7 +175,7 @@ print_errno_message (int errnum) #if defined HAVE_STRERROR_R || _LIBC char errbuf[1024]; -# if STRERROR_R_CHAR_P || _LIBC +# if _LIBC || STRERROR_R_CHAR_P s = __strerror_r (errnum, errbuf, sizeof errbuf); # else if (__strerror_r (errnum, errbuf, sizeof errbuf) == 0) @@ -124,7 +199,7 @@ print_errno_message (int errnum) #endif } -static void +static void _GL_ATTRIBUTE_FORMAT_PRINTF (3, 0) _GL_ARG_NONNULL ((3)) error_tail (int status, int errnum, const char *message, va_list args) { #if _LIBC @@ -165,7 +240,7 @@ error_tail (int status, int errnum, const char *message, va_list args) if (res != len) break; - if (__glibc_unlikely (len >= SIZE_MAX / sizeof (wchar_t) / 2)) + if (__builtin_expect (len >= SIZE_MAX / sizeof (wchar_t) / 2, 0)) { /* This really should not happen if everything is fine. */ res = (size_t) -1; @@ -227,7 +302,7 @@ error (int status, int errnum, const char *message, ...) 0); #endif - fflush (stdout); + flush_stdout (); #ifdef _LIBC _IO_flockfile (stderr); #endif @@ -273,6 +348,7 @@ error_at_line (int status, int errnum, const char *file_name, || (old_file_name != NULL && file_name != NULL && strcmp (old_file_name, file_name) == 0))) + /* Simply return and print nothing. */ return; @@ -288,7 +364,7 @@ error_at_line (int status, int errnum, const char *file_name, 0); #endif - fflush (stdout); + flush_stdout (); #ifdef _LIBC _IO_flockfile (stderr); #endif