[Bug 1001510] Fix compiler warnings about mismatch between log() format string and argument values.

[bugzilla-daemon@bugs.ecos.sourceware.org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001510

--- Comment #6 from Jonathan Larmour <jifl@ecoscentric.com> 2012-03-09 03:38:21 GMT ---
Created an attachment (id=1634)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1634)
time_t patch

(In reply to comment #3)
> > You're right.  Both fields in struct timeval are signed.  tv_sec is 'int' and
> > tv_usec is 'long int'.

I don't see that. The default is that in isoinfra, struct timeval is set to
contain two time_t's, which by default are ints. If POSIX is included, then
that is overridden[1] so it contains two longs. I don't see any mix of int and
long. Are you thinking of struct timespec instead?

[1] Using this in the POSIX CDL:
 requires         { CYGBLD_ISO_STRUCTTIMEVAL_HEADER == \
                             "<cyg/posix/sys/time.h>" }

> > Using the old format I don't get any compiler warnings
> > I think "%d.%ld" is probably more correct, but it's moot on 32-bit
> > architectures. Either way the output is somewhat misleading -- "%d.%06ld" is
> > probably the best answer.

Looking at the standard, the tv_sec should be a time_t, and tv_usec should be a
suseconds_t. I can see no minimum width requirements - only that suseconds_t
must be signed; and although I haven't seen it explicitly the usage of
(time_t)-1 including example code in the POSIX standard makes it clear that it
should be signed as well.

Indeed time_t can sometimes be 32-bits, but now increasingly 64-bits, to solve
the year 2038 problem: http://en.wikipedia.org/wiki/Year_2038_problem

Also see this:
http://en.wikipedia.org/wiki/Unix_time#Representing_the_number

> > It looks like tv_sec is always time_t (signed int), but depending on
> > CDL options, tv_usec can be time_t or unsigned long.  I'm now thinking
> > the right thing to do is to leave the format as %ld and cast each of
> > the parameters to (long).  Assuming the fields remain signed types,
> > that should work correctly regardless of whether they're (int) or
> > (long int).

Although we're not yet in a position to convert wholesale to 64-bit
time_t, using long would not be sufficient when we did - it would need to
be long long and so %lld.

But in the first instance, yes we could change time_t to a long. And I'm
attaching a patch to do that which I think I ought to commit regardless.
Comments?

Even so, the format specifier for time_t does happen to be a particular
problem. So for this file, how about instead using something like:

#define INTFORMAT(_t_) (sizeof(_t_) == sizeof(long long) ? "lld" : "ld")

log(LOG_DEBUG, "%s: = %" INTFORMAT(tp->tv_sec) ".%06"
INTFORMAT(tp->tv_usec) "\n", __FUNCTION__, (

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.