Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).

[Jasper Lievisse Adriaanse]

On Tue, Sep 04, 2012 at 09:20:24AM -0600, Eric Blake wrote:
> [adding bug-gnulib]
> 
> On 09/04/2012 08:49 AM, Jasper Lievisse Adriaanse wrote:
> >>From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001
> > From: Jasper Lievisse Adriaanse <address@hidden>
> > Date: Tue, 4 Sep 2012 16:47:26 +0200
> > Subject: [PATCH] Pass a correct pointer type to localtime_r(3).
> > 
> > Fixes a warning:
> > warning: passing argument 1 of 'localtime_r' from incompatible pointer type
> > ---
> >  tools/virsh-domain.c |    3 ++-
> >  tools/virsh.c        |    3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> NACK from the libvirt point of view.  tv_sec is required by POSIX to be
> of type time_t; so this is a bug in the OpenBSD header, and gnulib
> should be working around this bug.
OpenBSD's sys/time.h has this:

/*
 * Structure returned by gettimeofday(2) system call,
 * and used in other calls.
 */
struct timeval {
        long    tv_sec;         /* seconds */
        long    tv_usec;        /* and microseconds */
};

#ifndef _TIMESPEC_DECLARED
#define _TIMESPEC_DECLARED
/*
 * Structure defined by POSIX.1b to be like a timeval.
 */
struct timespec {
        time_t  tv_sec;         /* seconds */
        long    tv_nsec;        /* and nanoseconds */
};
#endif

> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index f0ec742..535779c 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -3711,6 +3711,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, 
> > const char *mime)
> >      struct tm time_info;
> >      const char *ext = NULL;
> >      char *ret = NULL;
> > +    time_t sec = (time_t) cur_time.tv_sec;
> >  
> >      if (!dom) {
> >          vshError(ctl, "%s", _("Invalid domain supplied"));
> > @@ -3724,7 +3725,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, 
> > const char *mime)
> >      /* add mime type here */
> >  
> >      gettimeofday(&cur_time, NULL);
> > -    localtime_r(&cur_time.tv_sec, &time_info);
> > +    localtime_r(&sec, &time_info);
> >      strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
> >  
> 
> > +++ b/tools/virsh.c
> > @@ -2189,6 +2189,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, 
> > const char *msg_format,
> >      const char *lvl = "";
> >      struct timeval stTimeval;
> >      struct tm *stTm;
> > +    time_t sec = stTimeval.tv_sec;
> >  
> >      if (ctl->log_fd == -1)
> >          return;
> > @@ -2199,7 +2200,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, 
> > const char *msg_format,
> >       * [YYYY.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
> >      */
> >      gettimeofday(&stTimeval, NULL);
> > -    stTm = localtime(&stTimeval.tv_sec);
> > +    stTm = localtime(&sec);
> 
> Even grosser - why is virsh using localtime() instead of localtime_r()?
Oversight probably..
 
> -- 
> Eric Blake   address@hidden    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 



-- 
Cheers,
Jasper

"Stay Hungry. Stay Foolish"