| Message ID | 1408618663-2281-18-git-send-email-siddhesh@redhat.com |
|---|---|
| State | New |
| Headers | show |
Please explain this more. There must be on appearances of IS_IN or anything like that in actual installed headers. So I guess you are talking about include/ wrappers for headers that will be installed. But still I don't understand under what conditions any of this code is used where _LIBC is not defined. Is it just in tests?
On Fri, Aug 22, 2014 at 12:38:54PM -0700, Roland McGrath wrote: > Please explain this more. There must be on appearances of IS_IN or > anything like that in actual installed headers. So I guess you are > talking about include/ wrappers for headers that will be installed. > But still I don't understand under what conditions any of this code > is used where _LIBC is not defined. Is it just in tests? This patch is wrong; I forgot to mention it here. I only ought to have made the change in the include wrappers and not the installed bits since the latter would break conformance tests. This is only needed in tests. I'll fix this up too. Siddhesh
On Fri, 22 Aug 2014, Roland McGrath wrote: > Please explain this more. There must be on appearances of IS_IN or > anything like that in actual installed headers. So I guess you are > talking about include/ wrappers for headers that will be installed. > But still I don't understand under what conditions any of this code > is used where _LIBC is not defined. Is it just in tests? bits/stdio-lock.h, as changed by this patch, (a) is installed; (b) includes the non-installed header <lowlevellock.h> (at least); (c) is not namespace-clean; (d) only actually gets included by libio.h if _IO_MTSAFE_IO is defined, which is not the case for any valid use of installed libc, so this doesn't cause any user-visible bugs. Of course that should still be cleaned up. A clean state would, as far as possible, not have the installed libio.h containing any code (such as _IO_MTSAFE_IO conditionals) that's not relevant for users of installed libc; that would go in include/libio.h instead. It would not have bits/stdio-lock.h installed, since it's not actually used in installed headers. That header would have a name that doesn't involve bits/, following the rule that bits/ headers are installed headers (see bug 14912). And it's doubtful if _IO_MTSAFE_IO (or _LIBC_REENTRANT) conditionals are needed at all - maybe the code should just assume it's true (outside installed headers, with the conditionals in installed headers having been moved to include/). (I'd think it would be better for any single-thread ports to work via macros that expand to do nothing, rather than having _IO_MTSAFE_IO or _LIBC_REENTRANT conditionals on calls to those macros.) (Before removing such conditionals, make sure the macro in question is actually defined for the source file in question - _IO_MTSAFE_IO isn't defined globally, but by lots of separate makefile references to $(libio-mtsafe).) libio also has lots of _LIBC conditionals that are probably relics of when it was used in libstdc++. In general such conditionals are unnecessary in files that are neither installed nor shared with other projects.
diff --git a/bits/stdio-lock.h b/bits/stdio-lock.h index f68b13e..937cc89 100644 --- a/bits/stdio-lock.h +++ b/bits/stdio-lock.h @@ -19,6 +19,10 @@ #ifndef _BITS_STDIO_LOCK_H #define _BITS_STDIO_LOCK_H 1 +#ifndef _LIBC +# define IS_IN(lib) (0) +#endif + #include <bits/libc-lock.h> __libc_lock_define_recursive (typedef, _IO_lock_t) @@ -44,7 +48,7 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) #define _IO_cleanup_region_end(_doit) \ __libc_cleanup_region_end (_doit) -#if defined _LIBC && IS_IN (libc) +#if IS_IN (libc) # define _IO_acquire_lock(_fp) \ _IO_cleanup_region_start ((void (*) (void *)) _IO_funlockfile, (_fp)); \ _IO_flockfile (_fp) diff --git a/include/mqueue.h b/include/mqueue.h index aba788e..182b7de 100644 --- a/include/mqueue.h +++ b/include/mqueue.h @@ -1,4 +1,7 @@ #include <rt/mqueue.h> +#ifndef _LIBC +# define IS_IN(lib) (0) +#endif #if IS_IN (librt) hidden_proto (mq_timedsend) diff --git a/include/stdlib.h b/include/stdlib.h index 734f251..fd80b6d 100644 --- a/include/stdlib.h +++ b/include/stdlib.h @@ -1,5 +1,9 @@ #ifndef _STDLIB_H +#ifndef _LIBC +# define IS_IN(lib) (0) +#endif + #ifdef __need_malloc_and_calloc #define __Need_M_And_C #endif diff --git a/sysdeps/nptl/bits/stdio-lock.h b/sysdeps/nptl/bits/stdio-lock.h index da392ed..6675a6d 100644 --- a/sysdeps/nptl/bits/stdio-lock.h +++ b/sysdeps/nptl/bits/stdio-lock.h @@ -19,6 +19,10 @@ #ifndef _BITS_STDIO_LOCK_H #define _BITS_STDIO_LOCK_H 1 +#ifndef _LIBC +# define IS_IN(lib) (0) +#endif + #include <bits/libc-lock.h> #include <lowlevellock.h> @@ -84,7 +88,7 @@ typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; #define _IO_cleanup_region_end(_doit) \ __libc_cleanup_region_end (_doit) -#if defined _LIBC && IS_IN (libc) +#if IS_IN (libc) # ifdef __EXCEPTIONS # define _IO_acquire_lock(_fp) \
This is useful for installed headers and also for headers that are used in tests without setting IN_MODULE. * bits/stdio-lock.h [!_LIBC]: Define IS_IN. * include/mqueue.h [!_LIBC]: Likewise. * include/stdlib.h [!_LIBC]: Likewise. * sysdeps/nptl/bits/stdio-lock.h [!_LIBC]: Likewise. --- bits/stdio-lock.h | 6 +++++- include/mqueue.h | 3 +++ include/stdlib.h | 4 ++++ sysdeps/nptl/bits/stdio-lock.h | 6 +++++- 4 files changed, 17 insertions(+), 2 deletions(-)