Sync up error.c with gnulib
Commit Message
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)
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.
Comments
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.
@@ -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