TIME_MAX not set correctly for OpenBSD in qemu-common.h

Looking at the OpenBSD buildbot logs I noticed a warning that appears to be a bug in the code.
OpenBSD has a 32-bit time_t on all archs at the moment (32-bit and 64-bit).

  CC    i386-softmmu/monitor.o
/buildbot-qemu/default_openbsd_current/build/monitor.c: In function 'expire_password':
/buildbot-qemu/default_openbsd_current/build/monitor.c:944: warning: overflow in implicit constant conversion

qemu-common.h has...

#ifndef TIME_MAX
#define TIME_MAX LONG_MAX
#endif

for OpenBSD this should be INT_MAX.

On 11/12/11 5:53 AM, Stefan Weil wrote:
> Am 11.12.2011 07:47, schrieb Brad Smith:
>> Public bug reported:
>>
>> Looking at the OpenBSD buildbot logs I noticed a warning that appears
>> to be a bug in the code.
>> OpenBSD has a 32-bit time_t on all archs at the moment (32-bit and
>> 64-bit).
>>
>> CC i386-softmmu/monitor.o
>> /buildbot-qemu/default_openbsd_current/build/monitor.c: In function
>> 'expire_password':
>> /buildbot-qemu/default_openbsd_current/build/monitor.c:944: warning:
>> overflow in implicit constant conversion
>>
>> qemu-common.h has...
>>
>> #ifndef TIME_MAX
>> #define TIME_MAX LONG_MAX
>> #endif
>>
>> for OpenBSD this should be INT_MAX.
>>
>> ** Affects: qemu
>> Importance: Undecided
>> Status: New
>
> This needs special handling for w32 / w64, too.
> Looking at the code where TIME_MAX is used, I assume that
> more fixes are needed. The following code for example
> won't work:
>
> if (lifetime > INT_MAX) {
>
> What about using
>
> #define TIME_FOREVER -1
>
> instead of TIME_MAX? Of course this would need additional
> code changes.
>
> Regards,
> Stefan Weil

Gerd?

Still looking for comment on this since you added the initial code which
has this bug in it.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.



  Hi,

>>> Looking at the OpenBSD buildbot logs I noticed a warning that appears
>>> to be a bug in the code.
>>> OpenBSD has a 32-bit time_t on all archs at the moment (32-bit and
>>> 64-bit).

Ouch.  Adding 64bit arch with 32bit time_t is pretty lame IMHO.  There
are a bunch of years left to fix that that though.

>>> #ifndef TIME_MAX
>>> #define TIME_MAX LONG_MAX
>>> #endif
>>>
>>> for OpenBSD this should be INT_MAX.

Guess we'll need an #ifdef then.

>> This needs special handling for w32 / w64, too.
>> Looking at the code where TIME_MAX is used, I assume that
>> more fixes are needed. The following code for example
>> won't work:
>>
>> if (lifetime > INT_MAX) {

With 32bit time_t lifetime wouldn't become larger than INT_MAX anyway,
so it doesn't matter ;)

> Still looking for comment on this since you added the initial code which
> has this bug in it.

cheers,
  Gerd


Triaging old bug tickets... can you still reproduce this issue with the latest version of QEMU? Or could we close this ticket nowadays?


Since this bug report was filed OpenBSD has switched from 32-bit time_t to 64-bit time_t on all archs (yes, including 32-bit archs like i386, arm, powerpc). So instead of INT_MAX TIME_MAX should now be set to LLONG_MAX.

This was fixed in commit e7b47c22e2df14d, which was in the 2.11.0 release.