Re: [PATCH 1/3] Add and use latest <sys/timespec.h> from FreeBSD

[Craig Howland <howland at LGSInnovations dot com>]

On 03/09/2015 09:03 AM, Sebastian Huber wrote:
> This addresses a potential multiple definition problem and makes it
> easier to use FreeBSD header files in Newlib.
>
> newlib/ChangeLog
> 2015-03-09  Sebastian Huber  <sebastian.huber@embedded-brains.de>
>
>          * libc/include/sys/_timespec.h: Import from FreeBSD.
>          * libc/include/sys/timespec.h: Likewise.
>          * libc/include/sys/types.h: Include <sys/timespec.h>.
>          (timespec): Delete.
>          (itimerspec): Likewise.
> ---
>   newlib/libc/include/sys/_timespec.h | 49 +++++++++++++++++++++++++++++
>   newlib/libc/include/sys/timespec.h  | 63 +++++++++++++++++++++++++++++++++++++
>   newlib/libc/include/sys/types.h     | 16 +---------
>   3 files changed, 113 insertions(+), 15 deletions(-)
>   create mode 100644 newlib/libc/include/sys/_timespec.h
>   create mode 100644 newlib/libc/include/sys/timespec.h
>
> diff --git a/newlib/libc/include/sys/_timespec.h b/newlib/libc/include/sys/_timespec.h
> ...
> +
> +#ifndef __time_t_defined
> +typedef	_TIME_T_	time_t;
> +#define	__time_t_defined
> +#endif
>
> diff --git a/newlib/libc/include/sys/timespec.h b/newlib/libc/include/sys/timespec.h
> ...
> +#include <sys/_timespec.h>
> ...
> diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h
> index ed33e0a..280de5f 100644
> --- a/newlib/libc/include/sys/types.h
> +++ b/newlib/libc/include/sys/types.h
> @@ -68,6 +68,7 @@ typedef	quad_t *	qaddr_t;
>   
>   # include <stddef.h>
>   # include <machine/types.h>
> +# include <sys/timespec.h>
>   
>   /* To ensure the stat struct's layout doesn't change when sizeof(int), etc.
>      changes, we assume sizeof short and long never change and have all types
> @@ -123,21 +124,6 @@ typedef _TIME_T_ time_t;
>   #define __time_t_defined
>   #endif
> ...
A minor nit:  the conditional typedef _TIME_T_ time_t in sys/types.h (just the 
end of which is seen in the patch) becomes degenerate--the if will never 
pass--because of the unconditional inclusion of sys/timespec.h that will define 
it first.  Is it perhaps worth removing it because it is somewhat misleading?  
Change it to a comment, or even leave it the way it is to make for less 
searching when trying to figure out where it really is defined?  Or, the 
#include <sys/timespec.h> could even be moved to the place in sys/types.h where 
the timespec definitions it replaces are defined (that is, right after the 
time_t typedef).  While it would work fine the way it is, it just seems to be 
not good practice to have dead code laying around.