diff mbox

osdep: Deal with TIME_MAX and OpenBSD 64-bit time_t

Message ID 20171106005656.GC84093@humpty.home.comstyle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Brad Smith Nov. 6, 2017, 12:56 a.m. UTC
Define TIME_MAX to LLONG_MAX for OpenBSD since OpenBSD uses 64-bit time_t.

Signed-off-by: Brad Smith <brad@comstyle.com>

Comments

Peter Maydell Nov. 6, 2017, 10:51 a.m. UTC | #1
On 6 November 2017 at 00:56, Brad Smith <brad@comstyle.com> wrote:
> Define TIME_MAX to LLONG_MAX for OpenBSD since OpenBSD uses 64-bit time_t.
>
> Signed-off-by: Brad Smith <brad@comstyle.com>
>
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 6855b94bbf..824714049b 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -132,8 +132,12 @@ extern int daemon(int, int);
>  #define ESHUTDOWN 4099
>  #endif
>  #ifndef TIME_MAX
> +#ifdef __OpenBSD__
> +#define TIME_MAX LLONG_MAX
> +#else
>  #define TIME_MAX LONG_MAX
>  #endif
> +#endif

I'm not really a fan of adding new OS-specific #ifdefs --
what if one of the other BSDs uses or switches to 64-bit
time_t for 32-bit platforms? Is there some way we can detect
this generically at compile time (possibly in configure) ?

thanks
-- PMM
Daniel P. Berrangé Nov. 6, 2017, 11:26 a.m. UTC | #2
On Mon, Nov 06, 2017 at 10:51:16AM +0000, Peter Maydell wrote:
> On 6 November 2017 at 00:56, Brad Smith <brad@comstyle.com> wrote:
> > Define TIME_MAX to LLONG_MAX for OpenBSD since OpenBSD uses 64-bit time_t.
> >
> > Signed-off-by: Brad Smith <brad@comstyle.com>
> >
> >
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index 6855b94bbf..824714049b 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -132,8 +132,12 @@ extern int daemon(int, int);
> >  #define ESHUTDOWN 4099
> >  #endif
> >  #ifndef TIME_MAX
> > +#ifdef __OpenBSD__
> > +#define TIME_MAX LLONG_MAX
> > +#else
> >  #define TIME_MAX LONG_MAX
> >  #endif
> > +#endif
> 
> I'm not really a fan of adding new OS-specific #ifdefs --
> what if one of the other BSDs uses or switches to 64-bit
> time_t for 32-bit platforms? Is there some way we can detect
> this generically at compile time (possibly in configure) ?

You could use a pair of compile time asserts to figure it out.

Would need one compile test to check 32 vs 64 bit:

  #include <time.h>
  char time_t_64bit[sizeof(time_t) == 8 ? 1 : -1];

and a second to check signed vs unsigned:

  #include <time.h>
  char time_t_signed[(time_t) -1 < 0 ? 1 : -1];


Save each of these programs to the file $TMPC, and then run 'compile_object'
from configure. You then have a decision matrix for 4 different TIME_MAX
values to write into config-host.h.

Regards,
Daniel
Kamil Rytarowski Nov. 6, 2017, 5:21 p.m. UTC | #3
On 06.11.2017 11:51, Peter Maydell wrote:
> On 6 November 2017 at 00:56, Brad Smith <brad@comstyle.com> wrote:
>> Define TIME_MAX to LLONG_MAX for OpenBSD since OpenBSD uses 64-bit time_t.
>>
>> Signed-off-by: Brad Smith <brad@comstyle.com>
>>
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index 6855b94bbf..824714049b 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -132,8 +132,12 @@ extern int daemon(int, int);
>>  #define ESHUTDOWN 4099
>>  #endif
>>  #ifndef TIME_MAX
>> +#ifdef __OpenBSD__
>> +#define TIME_MAX LLONG_MAX
>> +#else
>>  #define TIME_MAX LONG_MAX
>>  #endif
>> +#endif
> 
> I'm not really a fan of adding new OS-specific #ifdefs --
> what if one of the other BSDs uses or switches to 64-bit
> time_t for 32-bit platforms? Is there some way we can detect
> this generically at compile time (possibly in configure) ?
> 

NetBSD 32-bit ports switched to 64-bit in 2009. There are no longer
supported releases with 32-bit time_t.

> thanks
> -- PMM
>
Paolo Bonzini Nov. 6, 2017, 5:41 p.m. UTC | #4
On 06/11/2017 12:26, Daniel P. Berrange wrote:
> On Mon, Nov 06, 2017 at 10:51:16AM +0000, Peter Maydell wrote:
>> On 6 November 2017 at 00:56, Brad Smith <brad@comstyle.com> wrote:
>>> Define TIME_MAX to LLONG_MAX for OpenBSD since OpenBSD uses 64-bit time_t.
>>>
>>> Signed-off-by: Brad Smith <brad@comstyle.com>
>>>
>>>
>>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>>> index 6855b94bbf..824714049b 100644
>>> --- a/include/qemu/osdep.h
>>> +++ b/include/qemu/osdep.h
>>> @@ -132,8 +132,12 @@ extern int daemon(int, int);
>>>  #define ESHUTDOWN 4099
>>>  #endif
>>>  #ifndef TIME_MAX
>>> +#ifdef __OpenBSD__
>>> +#define TIME_MAX LLONG_MAX
>>> +#else
>>>  #define TIME_MAX LONG_MAX
>>>  #endif
>>> +#endif
>>
>> I'm not really a fan of adding new OS-specific #ifdefs --
>> what if one of the other BSDs uses or switches to 64-bit
>> time_t for 32-bit platforms? Is there some way we can detect
>> this generically at compile time (possibly in configure) ?
> 
> You could use a pair of compile time asserts to figure it out.
> 
> Would need one compile test to check 32 vs 64 bit:
> 
>   #include <time.h>
>   char time_t_64bit[sizeof(time_t) == 8 ? 1 : -1];
> 
> and a second to check signed vs unsigned:
> 
>   #include <time.h>
>   char time_t_signed[(time_t) -1 < 0 ? 1 : -1];
> 
> 
> Save each of these programs to the file $TMPC, and then run 'compile_object'
> from configure. You then have a decision matrix for 4 different TIME_MAX
> values to write into config-host.h.

What about

#define type_max(t)                   \
    ((t) -1 > 0                       \
     ? (t)~0                          \
     : (((t)1) <<                     \
         (sizeof(t) * 8 - 1)) - 1)    \

#define TIME_MAX type_max(time_t)

?  We don't need it to be a cpp constant, do we?  Or if we did, we could
assume it to be signed, it's enough for the three users.

Thanks,

Paolo
Peter Maydell Nov. 23, 2017, 3:31 p.m. UTC | #5
On 6 November 2017 at 17:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
> What about
>
> #define type_max(t)                   \
>     ((t) -1 > 0                       \
>      ? (t)~0                          \
>      : (((t)1) <<                     \
>          (sizeof(t) * 8 - 1)) - 1)    \
>
> #define TIME_MAX type_max(time_t)
>
> ?  We don't need it to be a cpp constant, do we?  Or if we did, we could
> assume it to be signed, it's enough for the three users.

GCC doesn't like this phrasing, it complains
qmp.c: In function ‘qmp_expire_password’:
qmp.c:334:89: error: integer overflow in expression [-Werror=overflow]

because of the shift into the sign bit. Instead we can borrow the
TYPE_MAXIMUM macro from gnulib, which uses basically the same
approach but avoids the UB and the compiler warning. I'll send
a patch in a bit.

thanks
-- PMM
diff mbox

Patch

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 6855b94bbf..824714049b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -132,8 +132,12 @@  extern int daemon(int, int);
 #define ESHUTDOWN 4099
 #endif
 #ifndef TIME_MAX
+#ifdef __OpenBSD__
+#define TIME_MAX LLONG_MAX
+#else
 #define TIME_MAX LONG_MAX
 #endif
+#endif
 
 /* HOST_LONG_BITS is the size of a native pointer in bits. */
 #if UINTPTR_MAX == UINT32_MAX