[RFC] y2038: test: Add _Static_assert() check when __USE_TIME_BITS64 is defined

Message ID 20201209145331.18819-1-lukma@denx.de
State Superseded
Headers
Series [RFC] y2038: test: Add _Static_assert() check when __USE_TIME_BITS64 is defined |

Commit Message

Lukasz Majewski Dec. 9, 2020, 2:53 p.m. UTC
  This check is added to exported time/time.h file to check in the compile
time if time_t and struct timespec have proper sizes and alignment.

It shall prevent from compiling user programs with -D_TIME_BITS=64 when
time related data structures are not supporting 64 bit time.
---
 time/time.h | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Paul Eggert Dec. 10, 2020, 12:42 a.m. UTC | #1
On 12/9/20 6:53 AM, Lukasz Majewski wrote:

> +#define CHECK_TIME64_SIZE(__name, __len) \
> +  _Static_assert (sizeof (__name) == __len, "Size of " #__name " != " #__len)
> +
> +#ifdef __USE_TIME_BITS64
> +  CHECK_TIME64_SIZE(time_t, 8);
> +  CHECK_TIME64_SIZE(struct timespec, 16);
> +#endif

I've lost context here; what branch is this against? glibc master doesn't 
have __USE_TIME_BITS64.

If this is in a publicly-visible file, the macro CHECK_TIME64_SIZE would 
need to have a reserved name.

I'm leery of the idea of putting checks like this into include files that 
users see. If there's a reason a platform cannot support 64-bit time_t even 
though __USE_TIME_BITS64 is defined, doesn't this sort of checking belong in 
the include file that defines time_t or __TIME_T_TYPE or whatever? That way, 
a user who sees the resulting diagnostic will have an easier time figuring 
out what exactly went wrong.
  
Lukasz Majewski Dec. 13, 2020, 11:48 a.m. UTC | #2
Hi Paul,

Thank you for the reply.

> On 12/9/20 6:53 AM, Lukasz Majewski wrote:
> 
> > +#define CHECK_TIME64_SIZE(__name, __len) \
> > +  _Static_assert (sizeof (__name) == __len, "Size of " #__name "
> > != " #__len) +
> > +#ifdef __USE_TIME_BITS64
> > +  CHECK_TIME64_SIZE(time_t, 8);
> > +  CHECK_TIME64_SIZE(struct timespec, 16);
> > +#endif  
> 
> I've lost context here; what branch is this against? glibc master
> doesn't have __USE_TIME_BITS64.

Yes, it doesn't (yet) support it. This will be available when
_TIME_BITS=64 is supported on ports with __WORDSIZE=32 &&
__TIMESIZE!=64.

> 
> If this is in a publicly-visible file, the macro CHECK_TIME64_SIZE
> would need to have a reserved name.

Ok. I see - we will stumble upon the reserved namespaces for POSIX
compliant exported headers.

> 
> I'm leery of the idea of putting checks like this into include files
> that users see.

The idea here is to prevent compiling user's program on machine, which
will not properly support 64 bit times - for example the underlying
glibc was very old. I do know that this is not preventing from all
threads - as one can just copy binary from some similar system.

It was suggested once (by Adhemerval and Andreas) that _Static_asserts
shall be added to glibc, but I'm a bit confused:

1. If the check is added to exported headers, it will (probably) pollute
the header itself (like in this patch).

2. I could add it into the glibc sources, but then I shall NOT use
__USE_TIME_BITS64 for enabling it, as this is #define from exported
header (and is not visible during glibc build itself). Instead, maybe I
should use __WORDSIZE==32 && __TIMESIZE!=64 as the condition?

> If there's a reason a platform cannot support 64-bit
> time_t even though __USE_TIME_BITS64 is defined, doesn't this sort of
> checking belong in the include file that defines time_t or
> __TIME_T_TYPE or whatever? That way, a user who sees the resulting
> diagnostic will have an easier time figuring out what exactly went
> wrong.

Interesting, thanks for the idea.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
  
Paul Eggert Dec. 15, 2020, 6:54 p.m. UTC | #3
On 12/13/20 3:48 AM, Lukasz Majewski wrote:

> Instead, maybe I
> should use __WORDSIZE==32 && __TIMESIZE!=64 as the condition?

Sounds reasonable. And that could be done with #if and so would generate 
better diagnostics for pre-C11 compilers (assuming this stuff is in an 
exported header).
  

Patch

diff --git a/time/time.h b/time/time.h
index 9a74f01b2f..d293ff3dc5 100644
--- a/time/time.h
+++ b/time/time.h
@@ -446,4 +446,12 @@  extern int getdate_r (const char *__restrict __string,
 
 __END_DECLS
 
+#define CHECK_TIME64_SIZE(__name, __len) \
+  _Static_assert (sizeof (__name) == __len, "Size of " #__name " != " #__len)
+
+#ifdef __USE_TIME_BITS64
+  CHECK_TIME64_SIZE(time_t, 8);
+  CHECK_TIME64_SIZE(struct timespec, 16);
+#endif
+
 #endif /* time.h.  */