[libvirt] [PATCH] time_t is not a long on FreeBSD, need to add casts

--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec)); if (def->name == NULL) { virReportOOMError(); @@ -9126,7 +9126,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, if (!newSnapshot) { if (virXPathLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + (long *)&def->creationTime) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; @@ -9192,7 +9192,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </parent>\n"); } virBufferAsprintf(&buf, " <creationTime>%ld</creationTime>\n", - def->creationTime); + (long)def->creationTime); virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); virBufferAddLit(&buf, " </domain>\n"); -- 1.7.0.4

On Fri, May 13, 2011 at 07:53:29AM +0200, Matthias Bolte wrote:
--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec));
if (def->name == NULL) { virReportOOMError(); @@ -9126,7 +9126,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
if (!newSnapshot) { if (virXPathLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + (long *)&def->creationTime) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; @@ -9192,7 +9192,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </parent>\n"); } virBufferAsprintf(&buf, " <creationTime>%ld</creationTime>\n", - def->creationTime); + (long)def->creationTime); virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); virBufferAddLit(&buf, " </domain>\n");
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 05/12/2011 11:53 PM, Matthias Bolte wrote:
--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec));
Ouch. Cygwin is thinking of making time_t a 64-bit entity, but since cygwin is 32-bit, that's larger than long. We need to use %lld and (long long) instead (or %jd and intmax_t). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/5/13 Eric Blake <eblake@redhat.com>:
On 05/12/2011 11:53 PM, Matthias Bolte wrote:
--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec));
Ouch. Cygwin is thinking of making time_t a 64-bit entity, but since cygwin is 32-bit, that's larger than long.
We need to use %lld and (long long) instead (or %jd and intmax_t).
Okay here are the two affected patches with long long instead of long. Matthias

On 05/14/2011 02:32 AM, Matthias Bolte wrote:
+ ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec));
Ouch. Cygwin is thinking of making time_t a 64-bit entity, but since cygwin is 32-bit, that's larger than long.
We need to use %lld and (long long) instead (or %jd and intmax_t).
Okay here are the two affected patches with long long instead of long.
From 1d72db6b2e25168c80de6fcd420ca182fb273a6f Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Fri, 13 May 2011 07:07:13 +0200 Subject: [PATCH] time_t is not a long on FreeBSD, need to add casts
Cast to long long to be on the safe side when time_t is a 64bit type. --- src/conf/domain_conf.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a0eb43e..946297a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9070,7 +9070,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%lld", (long long)tv.tv_sec));
This part's fine...
if (def->name == NULL) { virReportOOMError(); @@ -9080,13 +9080,16 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, def->description = virXPathString("string(./description)", ctxt);
if (!newSnapshot) { - if (virXPathLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + long long creationTime; + + if (virXPathLongLong("string(./creationTime)", ctxt, + &creationTime) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; }
+ def->creationTime = creationTime;
But here, we should add a check for overflow, if long long is larger than time_t, as in: if (def->creationTime != creationTime) { virDomainReportError(..., _("overflow")); }
0002-virsh-time_t-is-not-a-long-on-FreeBSD.patch
From 17cccebb1e35ae56f61802b796598dc16d8a4d02 Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Fri, 13 May 2011 08:31:03 +0200 Subject: [PATCH] virsh: time_t is not a long on FreeBSD
localtime_r expects time_t. --- tools/virsh.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 3baa015..62ffc8e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10406,7 +10406,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) char *doc = NULL; virDomainSnapshotPtr snapshot = NULL; char *state = NULL; - long creation; + long long creation_longlong; + time_t creation_time_t; char timestr[100]; struct tm time_info;
@@ -10465,10 +10466,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) state = virXPathString("string(/domainsnapshot/state)", ctxt); if (state == NULL) continue; - if (virXPathLong("string(/domainsnapshot/creationTime)", ctxt, - &creation) < 0) + if (virXPathLongLong("string(/domainsnapshot/creationTime)", ctxt, + &creation_longlong) < 0) continue; - localtime_r(&creation, &time_info); + creation_time_t = creation_longlong;
Another instance where we need to check for overflow.
+ localtime_r(&creation_time_t, &time_info); strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z", &time_info);
vshPrint(ctl, " %-20s %-25s %s\n", names[i], timestr, state); -- 1.7.0.4
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Matthias Bolte