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

[Brad Smith]

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.

[Brad Smith (t-brad-3)]

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.

[Gerd Hoffmann (kraxel-redhat)]

  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

[Thomas Huth (th-huth)]

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

[Brad Smith (t-brad-3)]

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.

[Peter Maydell (pmaydell)]

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