This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2 4/5] Post-cleanup 1: move libio.h back out of bits/.
On 05/02/2018 21:34, Zack Weinberg wrote:
> In this patch, libio.h moves back out of bits/ into the top level of
> the libio subdirectory, and is merged with libio/bits/libio-ldbl.h
> (which also used to be installed) and include/libio.h. Since almost
> no files include libio.h directly, this is quite straightforward.
>
> libio.h is now always used with _LIBC defined, so all of the _LIBC ||
> _GLIBCPP_USE_WCHAR_T conditionals are unnecessary. Similarly, the
> ifdef nest surrounding the definition of _IO_fwide_maybe_incompatible
> can collapse down to a single SHLIB_COMPAT check. I also took the
> opportunity to add some checks for configuration botches to libio.h.
>
> Installed stripped libraries are unchanged by this patch.
>
> The distinction between libio.h and libioP.h should be preserved as
> long as include/stdio.h needs to include libio.h, and that's entangled
> with _IO_MTSAFE_IO. It might be feasible to merge iolibio.h into
> libioP.h at this point, but I'm not sure whether it's worth the trouble.
LGTM with a possible extra cleanups decribed below.
>
> * libio/bits/libio.h: Move back to libio/libio.h and adjust
> multiple-include guard to match.
> Merge contents of libio/bits/libio-ldbl.h and include/libio.h
> into this file.
> Remove preprocessor conditionals that are always true and/or
> redundant to other preprocessor conditionals in the same nest.
> Include shlib-compat.h unconditionally.
> Error out if _LIBC is not defined, or if _ISOMAC is defined,
> or if _IO_MTSAFE_IO is defined but _IO_lock_t_defined is not
> defined after including stdio.h.
> Use __BEGIN_DECLS/__END_DECLS.
>
> * libio/bits/libio-ldbl.h, include/bits/libio.h: Delete file.
> * include/stdio.h, libio/iolibio.h, libio/libioP.h: Include
> libio.h as <libio/libio.h> rather than as <bits/libio.h>.
> ---
> include/bits/libio.h | 45 -----------------------
> include/stdio.h | 2 +-
> libio/bits/libio-ldbl.h | 29 ---------------
> libio/iolibio.h | 2 +-
> libio/{bits => }/libio.h | 96 ++++++++++++++++++++++++++++++++----------------
> libio/libioP.h | 2 +-
> 6 files changed, 67 insertions(+), 109 deletions(-)
> delete mode 100644 include/bits/libio.h
> delete mode 100644 libio/bits/libio-ldbl.h
> rename libio/{bits => }/libio.h (87%)
>
> diff --git a/include/bits/libio.h b/include/bits/libio.h
> deleted file mode 100644
> index 572395d5ffb..00000000000
> --- a/include/bits/libio.h
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -#if !defined _ISOMAC && defined _IO_MTSAFE_IO
> -# include <stdio-lock.h>
> -#endif
> -#include <libio/bits/libio.h>
> -
> -#ifndef _ISOMAC
> -#ifndef _LIBC_LIBIO_H
> -#define _LIBC_LIBIO_H
> -
> -libc_hidden_proto (__overflow)
> -libc_hidden_proto (__underflow)
> -libc_hidden_proto (__uflow)
> -libc_hidden_proto (__woverflow)
> -libc_hidden_proto (__wunderflow)
> -libc_hidden_proto (__wuflow)
> -libc_hidden_proto (_IO_free_backup_area)
> -libc_hidden_proto (_IO_free_wbackup_area)
> -libc_hidden_proto (_IO_padn)
> -libc_hidden_proto (_IO_putc)
> -libc_hidden_proto (_IO_sgetn)
> -libc_hidden_proto (_IO_vfprintf)
> -libc_hidden_proto (_IO_vfscanf)
> -
> -#ifdef _IO_MTSAFE_IO
> -# undef _IO_peekc
> -# undef _IO_flockfile
> -# undef _IO_funlockfile
> -# undef _IO_ftrylockfile
> -
> -# define _IO_peekc(_fp) _IO_peekc_locked (_fp)
> -# if _IO_lock_inexpensive
> -# define _IO_flockfile(_fp) \
> - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock)
> -# define _IO_funlockfile(_fp) \
> - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lock)
> -# else
> -# define _IO_flockfile(_fp) \
> - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_flockfile (_fp)
> -# define _IO_funlockfile(_fp) \
> - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_funlockfile (_fp)
> -# endif
> -#endif /* _IO_MTSAFE_IO */
> -
> -#endif
> -#endif
Ok.
> diff --git a/include/stdio.h b/include/stdio.h
> index 3b6da17d82f..f12b281b4d6 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -5,7 +5,7 @@
> # include <libio/stdio.h>
> # ifndef _ISOMAC
> # define _LIBC_STDIO_H 1
> -# include <bits/libio.h>
> +# include <libio/libio.h>
>
> /* Now define the internal interfaces. */
>
Ok.
> diff --git a/libio/bits/libio-ldbl.h b/libio/bits/libio-ldbl.h
> deleted file mode 100644
> index aa39353ca50..00000000000
> --- a/libio/bits/libio-ldbl.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/* -mlong-double-64 compatibility mode for libio functions.
> - Copyright (C) 2006-2018 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <http://www.gnu.org/licenses/>. */
> -
> -#ifndef _BITS_LIBIO_LDBL_H
> -#define _BITS_LIBIO_LDBL_H 1
> -
> -#ifndef _BITS_LIBIO_H
> -# error "Never include <bits/libio-ldbl.h> directly; use <libio.h> instead."
> -#endif
> -
> -__LDBL_REDIR_DECL (_IO_vfscanf)
> -__LDBL_REDIR_DECL (_IO_vfprintf)
> -
> -#endif
Ok.
> diff --git a/libio/iolibio.h b/libio/iolibio.h
> index 7814b1d4e56..52731b65f91 100644
> --- a/libio/iolibio.h
> +++ b/libio/iolibio.h
> @@ -2,7 +2,7 @@
> #define _IOLIBIO_H 1
>
> #include <stdio.h>
> -#include <bits/libio.h>
> +#include <libio/libio.h>
>
> /* These emulate stdio functionality, but with a different name
> (_IO_ungetc instead of ungetc), and using _IO_FILE instead of FILE. */
Ok.
> diff --git a/libio/bits/libio.h b/libio/libio.h
> similarity index 87%
> rename from libio/bits/libio.h
> rename to libio/libio.h
> index cefc2c855b9..ff67e18c182 100644
> --- a/libio/bits/libio.h
> +++ b/libio/libio.h
> @@ -25,11 +25,22 @@
> This exception applies to code released by its copyright holders
> in files containing the exception. */
>
> -#ifndef _BITS_LIBIO_H
> -#define _BITS_LIBIO_H 1
> +#ifndef _LIBIO_H
> +#define _LIBIO_H 1
> +
> +#ifndef _LIBC
> +# error "libio.h should only be included when building glibc itself"
> +#endif
> +#ifdef _ISOMAC
> +# error "libio.h should not be included under _ISOMAC"
> +#endif
Ok.
>
> #include <stdio.h>
>
> +#if defined _IO_MTSAFE_IO && !defined _IO_lock_t_defined
> +# error "Someone forgot to include stdio-lock.h"
> +#endif
> +
> #include <bits/_G_config.h>
> /* ALL of these should be defined in _G_config.h */
> #define _IO_fpos_t __fpos_t
> @@ -46,6 +57,10 @@
> #define _IO_wint_t wint_t
> #define _IO_va_list __gnuc_va_list
>
> +#include <shlib-compat.h>
> +
> +__BEGIN_DECLS
> +
> /* compatibility defines */
> #define _STDIO_USES_IOSTREAM
> #define _IO_UNIFIED_JUMPTABLES 1
> @@ -143,7 +158,6 @@ enum __codecvt_result
> __codecvt_noconv
> };
>
> -#if defined _LIBC || defined _GLIBCPP_USE_WCHAR_T
> /* The order of the elements in the following struct must match the order
> of the virtual functions in the libstdc++ codecvt class. */
> struct _IO_codecvt
> @@ -198,7 +212,6 @@ struct _IO_wide_data
>
> const struct _IO_jump_t *_wide_vtable;
> };
> -#endif
>
>
> #ifndef __cplusplus
> @@ -236,17 +249,10 @@ extern void _IO_cookie_init (struct _IO_cookie_file *__cfile, int __read_write,
> void *__cookie, _IO_cookie_io_functions_t __fns);
> #endif
>
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
I think you can cleanup the __cplusplus guards for _IO_FILE as well.
> -
> extern int __underflow (_IO_FILE *);
> -#if defined _LIBC || defined _GLIBCPP_USE_WCHAR_T
> extern _IO_wint_t __wunderflow (_IO_FILE *);
> extern _IO_wint_t __wuflow (_IO_FILE *);
> extern _IO_wint_t __woverflow (_IO_FILE *, _IO_wint_t);
> -#endif
>
> #if __GNUC__ >= 3
> # define _IO_BE(expr, res) __builtin_expect ((expr), res)
> @@ -261,7 +267,6 @@ extern _IO_wint_t __woverflow (_IO_FILE *, _IO_wint_t);
> : *(unsigned char *) (_fp)->_IO_read_ptr)
> #define _IO_putc_unlocked(_ch, _fp) __putc_unlocked_body (_ch, _fp)
>
> -#if defined _LIBC || defined _GLIBCPP_USE_WCHAR_T
> # define _IO_getwc_unlocked(_fp) \
> (_IO_BE ((_fp)->_wide_data == NULL \
> || ((_fp)->_wide_data->_IO_read_ptr \
> @@ -273,7 +278,6 @@ extern _IO_wint_t __woverflow (_IO_FILE *, _IO_wint_t);
> >= (_fp)->_wide_data->_IO_write_end), 0) \
> ? __woverflow (_fp, _wch) \
> : (_IO_wint_t) (*(_fp)->_wide_data->_IO_write_ptr++ = (_wch)))
> -#endif
>
> #define _IO_feof_unlocked(_fp) __feof_unlocked_body (_fp)
> #define _IO_ferror_unlocked(_fp) __ferror_unlocked_body (_fp)
> @@ -319,28 +323,25 @@ extern _IO_off64_t _IO_seekpos (_IO_FILE *, _IO_off64_t, int);
>
> extern void _IO_free_backup_area (_IO_FILE *) __THROW;
>
> -#if defined _LIBC || defined _GLIBCPP_USE_WCHAR_T
> +
> extern _IO_wint_t _IO_getwc (_IO_FILE *__fp);
> extern _IO_wint_t _IO_putwc (wchar_t __wc, _IO_FILE *__fp);
> extern int _IO_fwide (_IO_FILE *__fp, int __mode) __THROW;
> -# if __GNUC__ >= 2
> +
> /* While compiling glibc we have to handle compatibility with very old
> versions. */
> -# if defined _LIBC && defined SHARED
> -# include <shlib-compat.h>
> -# if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> -# define _IO_fwide_maybe_incompatible \
> +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)
> +# define _IO_fwide_maybe_incompatible \
> (__builtin_expect (&_IO_stdin_used == NULL, 0))
> extern const int _IO_stdin_used;
> weak_extern (_IO_stdin_used);
> -# endif
> -# endif
> -# ifndef _IO_fwide_maybe_incompatible
> -# define _IO_fwide_maybe_incompatible (0)
> -# endif
> +#else
> +# define _IO_fwide_maybe_incompatible (0)
> +#endif
> +
> /* A special optimized version of the function above. It optimizes the
> case of initializing an unoriented byte stream. */
> -# define _IO_fwide(__fp, __mode) \
> +#define _IO_fwide(__fp, __mode) \
> ({ int __result = (__mode); \
> if (__result < 0 && ! _IO_fwide_maybe_incompatible) \
> { \
> @@ -354,7 +355,6 @@ weak_extern (_IO_stdin_used);
> else \
> __result = _IO_fwide (__fp, __result); \
> __result; })
> -# endif
>
> extern int _IO_vfwscanf (_IO_FILE * __restrict, const wchar_t * __restrict,
> _IO_va_list, int *__restrict);
> @@ -362,14 +362,46 @@ extern int _IO_vfwprintf (_IO_FILE *__restrict, const wchar_t *__restrict,
> _IO_va_list);
> extern _IO_ssize_t _IO_wpadn (_IO_FILE *, wint_t, _IO_ssize_t);
> extern void _IO_free_wbackup_area (_IO_FILE *) __THROW;
> -#endif
>
> #ifdef __LDBL_COMPAT
> -# include <bits/libio-ldbl.h>
> +__LDBL_REDIR_DECL (_IO_vfscanf)
> +__LDBL_REDIR_DECL (_IO_vfprintf)
> #endif
>
> -#ifdef __cplusplus
> -}
> -#endif
> +libc_hidden_proto (__overflow)
> +libc_hidden_proto (__underflow)
> +libc_hidden_proto (__uflow)
> +libc_hidden_proto (__woverflow)
> +libc_hidden_proto (__wunderflow)
> +libc_hidden_proto (__wuflow)
> +libc_hidden_proto (_IO_free_backup_area)
> +libc_hidden_proto (_IO_free_wbackup_area)
> +libc_hidden_proto (_IO_padn)
> +libc_hidden_proto (_IO_putc)
> +libc_hidden_proto (_IO_sgetn)
> +libc_hidden_proto (_IO_vfprintf)
> +libc_hidden_proto (_IO_vfscanf)
> +
> +#ifdef _IO_MTSAFE_IO
> +# undef _IO_peekc
> +# undef _IO_flockfile
> +# undef _IO_funlockfile
> +# undef _IO_ftrylockfile
> +
> +# define _IO_peekc(_fp) _IO_peekc_locked (_fp)
> +# if _IO_lock_inexpensive
> +# define _IO_flockfile(_fp) \
> + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock)
> +# define _IO_funlockfile(_fp) \
> + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lock)
> +# else
> +# define _IO_flockfile(_fp) \
> + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_flockfile (_fp)
> +# define _IO_funlockfile(_fp) \
> + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_funlockfile (_fp)
> +# endif
> +#endif /* _IO_MTSAFE_IO */
> +
> +__END_DECLS
>
> -#endif /* _BITS_LIBIO_H */
> +#endif /* _LIBIO_H */
As a side note, I think we can cleanup some definitions required to build it
externally:
271 #if __GNUC__ >= 3
272 # define _IO_BE(expr, res) __builtin_expect ((expr), res)
273 #else
274 # define _IO_BE(expr, res) (expr)
275 #endif
And just use __glibc_{un}likely instead.
Similar for:
122 #ifdef _LIBC
123 # define _IO_FLAGS2_FORTIFY 4
124 #endif
125 #define _IO_FLAGS2_USER_WBUF 8
126 #ifdef _LIBC
127 # define _IO_FLAGS2_SCANF_STD 16
128 # define _IO_FLAGS2_NOCLOSE 32
129 # define _IO_FLAGS2_CLOEXEC 64
130 # define _IO_FLAGS2_NEED_LOCK 128
131 #endif
and
240 #ifndef _LIBC
241 #define _IO_stdin ((_IO_FILE*)(&_IO_2_1_stdin_))
242 #define _IO_stdout ((_IO_FILE*)(&_IO_2_1_stdout_))
243 #define _IO_stderr ((_IO_FILE*)(&_IO_2_1_stderr_))
244 #else
245 extern _IO_FILE *_IO_stdin attribute_hidden;
246 extern _IO_FILE *_IO_stdout attribute_hidden;
247 extern _IO_FILE *_IO_stderr attribute_hidden;
248 #endif
Since there is already a guard for _LIBC definition.
Also:
252 #ifdef __USE_GNU
253 typedef cookie_read_function_t __io_read_fn;
254 typedef cookie_write_function_t __io_write_fn;
255 typedef cookie_seek_function_t __io_seek_fn;
256 typedef cookie_close_function_t __io_close_fn;
257 typedef cookie_io_functions_t _IO_cookie_io_functions_t;
258
259 struct _IO_cookie_file;
260
261 /* Initialize one of those. */
262 extern void _IO_cookie_init (struct _IO_cookie_file *__cfile, int __read_write,
263 void *__cookie, _IO_cookie_io_functions_t __fns);
264 #endif
Since for glibc build __USE_GNU is implicit.
> diff --git a/libio/libioP.h b/libio/libioP.h
> index ac66f95f8d4..8edc207277e 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -43,7 +43,7 @@
> #include <math_ldbl_opt.h>
>
> #include <stdio.h>
> -#include <bits/libio.h>
> +#include <libio/libio.h>
> #include "iolibio.h"
>
> #ifdef __cplusplus
>
Ok.