This is the mail archive of the
ecos-bugs@sourceware.org
mailing list for the eCos project.
[Bug 1001510] Fix compiler warnings about mismatch between log()format string and argument values.
- From: bugzilla-daemon at bugs dot ecos dot sourceware dot org
- To: unassigned at bugs dot ecos dot sourceware dot org
- Date: Fri, 9 Mar 2012 03:38:28 +0000
- Subject: [Bug 1001510] Fix compiler warnings about mismatch between log()format string and argument values.
- Auto-submitted: auto-generated
- References: <bug-1001510-777@http.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.