[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3).
From: |
Jasper Lievisse Adriaanse |
Subject: |
Re: [libvirt] [PATCH] Pass a correct pointer type to localtime_r(3). |
Date: |
Tue, 4 Sep 2012 19:29:43 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Sep 04, 2012 at 10:03:41AM -0700, Paul Eggert wrote:
> On 09/04/2012 08:20 AM, Eric Blake wrote:
> > tv_sec is required by POSIX to be
> > of type time_t; so this is a bug in the OpenBSD header
>
> Most likely this problem arose because of the patch I pushed
> in gnulib commit e07d7c40f3ca5ec410cf5aa6fa03cfe51e712039.
> Previously, gnulib required timeval's tv_sec to be
> the same size as time_t. But now, it requires only that
> tv_sec be big enough to hold a time_t.
>
> This patch was needed for Emacs. Without the patch, gnulib
> replaced struct timeval on OpenBSD, and this messed up
> utimens.c, and Emacs wouldn't build.
>
> Alternatively, gnulib could substitute its own struct timeval
> for the system's, wrapping every struct timeval-using function
> (gettimeofday, futimesat, futimes, lutimes, etc. That'd be
> more work, though. And it would introduce some performance
> issues with gettimeofday, which is supposed to be fast.
>
> I've been trying to get away from using struct timeval,
> and to use the higher-resolution struct timespec instead,
> so messing with these obsolescent interfaces has been
> lower priority for me. But if someone wants to take the
> more-ambitious approach that'd be fine, I expect.
>
> For this particular case, though, how about if we avoid
> the problem entirely? libvirt doesn't need to use struct
> timeval here at all. I'd use the following (untested) patch:
> it makes libvirt smaller and probably faster, and it ports
> to OpenBSD without messing with gnulib:
Works fine here on OpenBSD.
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index f0ec742..3cef782 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -3707,7 +3707,7 @@ static char *
> vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
> {
> char timestr[100];
> - struct timeval cur_time;
> + time_t cur_time;
> struct tm time_info;
> const char *ext = NULL;
> char *ret = NULL;
> @@ -3723,8 +3723,8 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const
> char *mime)
> ext = ".png";
> /* add mime type here */
>
> - gettimeofday(&cur_time, NULL);
> - localtime_r(&cur_time.tv_sec, &time_info);
> + time (&cur_time);
> + localtime_r(&cur_time, &time_info);
> strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
>
> if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom),
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 5cf3237..88da429 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2187,7 +2187,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const
> char *msg_format,
> char *str;
> size_t len;
> const char *lvl = "";
> - struct timeval stTimeval;
> + time_t stTime;
> struct tm *stTm;
>
> if (ctl->log_fd == -1)
> @@ -2198,8 +2198,8 @@ 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);
> + time (&stTime);
> + stTm = localtime(&stTime);
> virBufferAsprintf(&buf, "[%d.%02d.%02d %02d:%02d:%02d %s %d] ",
> (1900 + stTm->tm_year),
> (1 + stTm->tm_mon),
>
--
Cheers,
Jasper
"Stay Hungry. Stay Foolish"