This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH 08/13] Installed-header hygiene (BZ#20366): time.h types.
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Zack Weinberg <zackw at panix dot com>, libc-alpha at sourceware dot org
- Cc: joseph at codesourcery dot com
- Date: Wed, 21 Sep 2016 15:04:04 -0400
- Subject: Re: [PATCH 08/13] Installed-header hygiene (BZ#20366): time.h types.
- Authentication-results: sourceware.org; auth=none
- References: <email@example.com> <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org> <email@example.com> <firstname.lastname@example.org> <email@example.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>
> -#include <time.h>
Your change unconditionally pulls in the two new headers?
Why aren't the original semantics OK?
# include <bits/types/time_t.h>
# include <bits/types/struct_timespec.h>
# include <time.h>