Re: [Qemu-devel] [RFC PATCH V4 2/5] qapi: add event helper functions

[Eric Blake]

On 03/27/2014 02:03 AM, Wenchao Xia wrote:
> This file holds some functions that do not need to be generated.
> 
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
>  include/qapi/qmp-event.h |   27 +++++++++++++++++
>  qapi/Makefile.objs       |    1 +
>  qapi/qmp-event.c         |   70 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 0 deletions(-)
>  create mode 100644 include/qapi/qmp-event.h
>  create mode 100644 qapi/qmp-event.c
> 

> +    err = qemu_gettimeofday(&tv);
> +    if (err < 0) {
> +        /* Put -1 to indicate failure of getting host time */
> +        tv.tv_sec = -1;
> +        tv.tv_usec = -1;

You fixed the problem with C promotion here, but...

> +    }
> +
> +    obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
> +                             "'microseconds': %" PRId64 " }",
> +                             (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);

...here, C promotion rules bite once again :(  If tv_usec is uint32_t,
then it zero-extends rather than sign-extends into int64_t, and you may
end up with 0xffffffff instead of the intended -1.  When doing
potentially widening casts, it is only safe if you know the signedness
of the pre-cast value; but with struct timeval, POSIX doesn't make that
easy.

Maybe it's easier to just rewrite things with known types:

int64_t sec;
int usec;
qemu_timval tv;
err = qemu_gettimeofday(&tv);
if (err < 0) {
    sec = -1;
    usec = -1;
} else {
    sec = tv.tv_sec;
    usec = tv.tv_usec;
}
qobject_from_jsonf("... %"PRId64 ", ...%d", sec, usec)

since 'int' is guaranteed to be large enough for all the usec values we
care about on all platforms we compile on (that is, we require 32-bit
int, even if C allows for a 16-bit int implementation).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature