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

2012-09-04 Thread Jasper Lievisse Adriaanse 
From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001
From: Jasper Lievisse Adriaanse jas...@humppa.nl
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(-)

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);
 
 if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom),
diff --git a/tools/virsh.c b/tools/virsh.c
index 5cf3237..5be2a3c 100644
--- a/tools/virsh.c
+++ 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,
  * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
 */
 gettimeofday(stTimeval, NULL);
-stTm = localtime(stTimeval.tv_sec);
+stTm = localtime(sec);
 virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] ,
   (1900 + stTm-tm_year),
   (1 + stTm-tm_mon),
-- 
1.7.6


-- 
Cheers,
Jasper

Stay Hungry. Stay Foolish

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Jasper Lievisse Adriaanse 
FYI, these patches and those I will send later supersede the previous patchsets
I sent yesterday for fixing and making libvirt work on OpenBSD.

On Tue, Sep 04, 2012 at 04:49:52PM +0200, Jasper Lievisse Adriaanse wrote:
 From b53dc971cc50b5ac397e4568449d25041477c8d6 Mon Sep 17 00:00:00 2001
 From: Jasper Lievisse Adriaanse jas...@humppa.nl
 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(-)
 
 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);
  
  if (virAsprintf(ret, %s-%s%s, virDomainGetName(dom),
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 5cf3237..5be2a3c 100644
 --- a/tools/virsh.c
 +++ 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,
   * [.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
  */
  gettimeofday(stTimeval, NULL);
 -stTm = localtime(stTimeval.tv_sec);
 +stTm = localtime(sec);
  virBufferAsprintf(buf, [%d.%02d.%02d %02d:%02d:%02d %s %d] ,
(1900 + stTm-tm_year),
(1 + stTm-tm_mon),
 -- 
 1.7.6
 
 
 -- 
 Cheers,
 Jasper
 
 Stay Hungry. Stay Foolish
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list

-- 
Cheers,
Jasper

Stay Hungry. Stay Foolish

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Eric Blake 
[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 jas...@humppa.nl
 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.

 
 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,
   * [.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()?

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread 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 jas...@humppa.nl
  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 {
longtv_sec; /* seconds */
longtv_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 */
longtv_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,
* [.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   ebl...@redhat.com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 



-- 
Cheers,
Jasper

Stay Hungry. Stay Foolish

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Eric Blake 
On 09/04/2012 10:32 AM, Jasper Lievisse Adriaanse wrote:
 On Tue, Sep 04, 2012 at 09:20:24AM -0600, Eric Blake wrote:
 [adding bug-gnulib]


 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 {
   longtv_sec; /* seconds */
   longtv_usec;/* and microseconds */

Buggy.  But thanks for alerting us to the issue.  POSIX requires:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_time.h.html#tag_13_64

time_t tv_sec  Seconds.
suseconds_ttv_usec Microseconds.


 +stTm = localtime(sec);

 Even grosser - why is virsh using localtime() instead of localtime_r()?
 Oversight probably..

Yeah, but that's pre-existing oversight in libvirt, and shouldn't affect
gnulib.  Libvirt has a syntax check rule which is supposed to avoid
localtime() in favor of localtime_r(); I'll have to check why it isn't
firing properly.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Paul Eggert 
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:

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,
  *
  * [.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),

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Jasper Lievisse Adriaanse 
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,
   *
   * [.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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2012-09-04 Thread Eric Blake 
On 09/04/2012 11:03 AM, Paul Eggert wrote:
 
 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:

Looks similar to Jasper's original proposal, but with the benefit of
avoiding the buggy 'struct timeval' altogether.  ACK and pushed.  Paul,
I listed you in AUTHORS; if you have any preference for an alternate
name or email listing, let me know.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list