Re: [PATCH 08/13] Installed-header hygiene (BZ#20366): time.h types.

[Carlos O'Donell <carlos at redhat dot com>]

On 08/29/2016 09:16 PM, Zack Weinberg wrote:
> Many headers are expected to expose a subset of the type definitions
> in time.h.  time.h has a whole bunch of messy logic for conditionally
> defining some its types and structs, but, as best I can tell, this
> has never worked 100%.  In particular, __need_timespec is ineffective
> if _TIME_H has already been defined, which means that if you compile
> 
>   #include <time.h>
>   #include <sched.h>
> 
> with e.g. -fsyntax-only -std=c89 -Wall -Wsystem-headers, you will get
> 
> In file included from test.c:2:0:
> /usr/include/sched.h:74:57: warning: "struct timespec" declared inside
>   parameter list will not be visible outside of this definition or declaration
>  extern int sched_rr_get_interval (__pid_t __pid, struct timespec *__t) __THROW;
>                                                          ^~~~~~~~
> 
> And if you want to _use_ sched_rr_get_interval in a TU compiled that
> way, you're hosed.
> 
> Rather than make the logic in time.h even messier, I had been kicking
> around in my head the idea of replacing the __need/__defined mechanism
> with a set of small headers, bits/types/foo_t.h for every foo_t that
> more than one header may need to define.  This seemed like a good
> place to give it a try.  I rather like the effect; time.h and
> bits/time.h are now *much* simpler, and a lot of other headers are
> slightly simpler.

+1 This is a good way forward.

> This is NOT a complete conversion; I have only done the types formerly
> accessed by defining __need_something and then including either time.h
> or bits/time.h.  It should be sufficient as a proof of concept, though.
 
LGTM with the caveat that I have one nit/question below.

> diff --git a/posix/sched.h b/posix/sched.h
> index 253c963..5b431c6 100644
> --- a/posix/sched.h
> +++ b/posix/sched.h
> @@ -25,13 +25,14 @@
>  #include <bits/types.h>
>  
>  #define __need_size_t
> +#define __need_NULL
>  #include <stddef.h>
>  
> -#ifdef __USE_XOPEN2K
> -# define __need_time_t
> -# define __need_timespec
> +#include <bits/types/time_t.h>
> +#include <bits/types/struct_timespec.h>
> +#ifndef __USE_XOPEN2K
> +# include <time.h>
>  #endif
> -#include <time.h>

Your change unconditionally pulls in the two new headers?

Why aren't the original semantics OK?

e.g.

#ifdef __USE_XOPEN2K
# include <bits/types/time_t.h>
# include <bits/types/struct_timespec.h>
#else
# include <time.h>
#endif

-- 
Cheers,
Carlos.