Introduce a vastly simpler VIR_INT64_STR_BUFLEN constant which is large enough for all cases where we currently use INT_BUFSIZE_BOUND. This eliminates most use of the gnulib intprops.h header. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/hyperv/hyperv_driver.c | 3 +- src/nwfilter/nwfilter_ebiptables_driver.c | 35 +++++++++-------------- src/nwfilter/nwfilter_learnipaddr.c | 14 ++++----- src/util/virfile.c | 9 +++--- src/util/virhostcpu.c | 3 +- src/util/virlog.c | 3 +- src/util/virnetdevbridge.c | 10 +++---- src/util/virpidfile.c | 7 ++--- src/util/virstring.h | 2 ++ tests/virsystemdtest.c | 5 ++-- tools/virsh-domain-monitor.c | 3 +- tools/virt-login-shell.c | 7 +++-- 12 files changed, 42 insertions(+), 59 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index c9d22ec7c4..4677a25ff8 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -36,7 +36,6 @@ #include "openwsman.h" #include "virstring.h" #include "virkeycode.h" -#include "intprops.h" #define VIR_FROM_THIS VIR_FROM_HYPERV @@ -1339,7 +1338,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, Msvm_Keyboard *keyboard = NULL; virBuffer query = VIR_BUFFER_INITIALIZER; hypervInvokeParamsListPtr params = NULL; - char keycodeStr[INT_BUFSIZE_BOUND(int)]; + char keycodeStr[VIR_INT64_STR_BUFLEN]; virCheckFlags(0, -1); diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index eec1414023..f4c192aebb 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -40,7 +40,6 @@ #include "virfile.h" #include "vircommand.h" #include "configmake.h" -#include "intprops.h" #include "virstring.h" #include "virfirewall.h" @@ -820,10 +819,9 @@ iptablesHandleIPHdr(virFirewallPtr fw, bool directionIn, bool *skipRule, bool *skipMatch) { - char ipaddr[INET6_ADDRSTRLEN], - ipaddralt[INET6_ADDRSTRLEN], - number[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))]; + char ipaddr[INET6_ADDRSTRLEN]; + char ipaddralt[INET6_ADDRSTRLEN]; + char number[VIR_INT64_STR_BUFLEN]; const char *src = "--source"; const char *dst = "--destination"; const char *srcrange = "--src-range"; @@ -968,8 +966,7 @@ iptablesHandleIPHdrAfterStateMatch(virFirewallPtr fw, ipHdrDataDefPtr ipHdr, bool directionIn) { - char number[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))]; + char number[VIR_INT64_STR_BUFLEN]; char str[MAX_IPSET_NAME_LENGTH]; if (HAS_ENTRY_ITEM(&ipHdr->dataIPSet) && @@ -1152,10 +1149,8 @@ _iptablesCreateRuleInstance(virFirewallPtr fw, bool maySkipICMP) { char chain[MAX_CHAINNAME_LENGTH]; - char number[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))]; - char numberalt[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))]; + char number[VIR_INT64_STR_BUFLEN]; + char numberalt[VIR_INT64_STR_BUFLEN]; const char *target; bool srcMacSkipped = false; bool skipRule = false; @@ -1789,16 +1784,14 @@ ebtablesCreateRuleInstance(virFirewallPtr fw, virNWFilterVarCombIterPtr vars, bool reverse) { - char macaddr[VIR_MAC_STRING_BUFLEN], - ipaddr[INET_ADDRSTRLEN], - ipmask[INET_ADDRSTRLEN], - ipv6addr[INET6_ADDRSTRLEN], - number[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))], - numberalt[MAX(INT_BUFSIZE_BOUND(uint32_t), - INT_BUFSIZE_BOUND(int))], - field[MAX(VIR_MAC_STRING_BUFLEN, INET6_ADDRSTRLEN)], - fieldalt[MAX(VIR_MAC_STRING_BUFLEN, INET6_ADDRSTRLEN)]; + char macaddr[VIR_MAC_STRING_BUFLEN]; + char ipaddr[INET_ADDRSTRLEN]; + char ipmask[INET_ADDRSTRLEN]; + char ipv6addr[INET6_ADDRSTRLEN]; + char number[VIR_INT64_STR_BUFLEN]; + char numberalt[VIR_INT64_STR_BUFLEN]; + char field[VIR_INT64_STR_BUFLEN]; + char fieldalt[VIR_INT64_STR_BUFLEN]; char chain[MAX_CHAINNAME_LENGTH]; const char *target; bool hasMask = false; diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index 46ef65401c..5791724cf4 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -39,7 +39,6 @@ #include "internal.h" -#include "intprops.h" #include "virbuffer.h" #include "viralloc.h" #include "virlog.h" @@ -59,10 +58,6 @@ VIR_LOG_INIT("nwfilter.nwfilter_learnipaddr"); -#define IFINDEX2STR(VARNAME, ifindex) \ - char VARNAME[INT_BUFSIZE_BOUND(ifindex)]; \ - g_snprintf(VARNAME, sizeof(VARNAME), "%d", ifindex); - #define PKT_TIMEOUT_MS 500 /* ms */ /* structure of an ARP request/reply message */ @@ -239,7 +234,7 @@ static int virNWFilterRegisterLearnReq(virNWFilterIPAddrLearnReqPtr req) { int res = -1; - IFINDEX2STR(ifindex_str, req->ifindex); + g_autofree char *ifindex_str = g_strdup_printf("%d", req->ifindex); virMutexLock(&pendingLearnReqLock); @@ -260,6 +255,7 @@ virNWFilterTerminateLearnReq(const char *ifname) int rc = -1; int ifindex; virNWFilterIPAddrLearnReqPtr req; + g_autofree char *ifindex_str = NULL; /* It's possible that it's already been removed as a result of * virNWFilterDeregisterLearnReq during learnIPAddressThread() exit @@ -274,7 +270,7 @@ virNWFilterTerminateLearnReq(const char *ifname) return rc; } - IFINDEX2STR(ifindex_str, ifindex); + ifindex_str = g_strdup_printf("%d", ifindex); virMutexLock(&pendingLearnReqLock); @@ -294,7 +290,7 @@ bool virNWFilterHasLearnReq(int ifindex) { void *res; - IFINDEX2STR(ifindex_str, ifindex); + g_autofree char *ifindex_str = g_strdup_printf("%d", ifindex); virMutexLock(&pendingLearnReqLock); @@ -319,7 +315,7 @@ static virNWFilterIPAddrLearnReqPtr virNWFilterDeregisterLearnReq(int ifindex) { virNWFilterIPAddrLearnReqPtr res; - IFINDEX2STR(ifindex_str, ifindex); + g_autofree char *ifindex_str = g_strdup_printf("%d", ifindex); virMutexLock(&pendingLearnReqLock); diff --git a/src/util/virfile.c b/src/util/virfile.c index 5acac85bb9..6eaabf5371 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -68,7 +68,6 @@ #endif #include "configmake.h" -#include "intprops.h" #include "viralloc.h" #include "vircommand.h" #include "virerror.h" @@ -4067,7 +4066,7 @@ virFileReadValueInt(int *value, const char *format, ...) if (!virFileExists(path)) return -2; - if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &str) < 0) return -1; virStringTrimOptionalNewline(str); @@ -4107,7 +4106,7 @@ virFileReadValueUint(unsigned int *value, const char *format, ...) if (!virFileExists(path)) return -2; - if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &str) < 0) return -1; virStringTrimOptionalNewline(str); @@ -4147,7 +4146,7 @@ virFileReadValueUllong(unsigned long long *value, const char *format, ...) if (!virFileExists(path)) return -2; - if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &str) < 0) return -1; virStringTrimOptionalNewline(str); @@ -4188,7 +4187,7 @@ virFileReadValueScaledInt(unsigned long long *value, const char *format, ...) if (!virFileExists(path)) return -2; - if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0) + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &str) < 0) return -1; virStringTrimOptionalNewline(str); diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 7f14340f49..f3adc1b4ae 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -43,7 +43,6 @@ #include "virhostcpupriv.h" #include "physmem.h" #include "virerror.h" -#include "intprops.h" #include "virarch.h" #include "virfile.h" #include "virtypedparam.h" @@ -775,7 +774,7 @@ virHostCPUGetStatsLinux(FILE *procstat, char line[1024]; unsigned long long usr, ni, sys, idle, iowait; unsigned long long irq, softirq, steal, guest, guest_nice; - char cpu_header[4 + INT_BUFSIZE_BOUND(cpuNum)]; + char cpu_header[4 + VIR_INT64_STR_BUFLEN]; if ((*nparams) == 0) { /* Current number of cpu stats supported by linux */ diff --git a/src/util/virlog.c b/src/util/virlog.c index 8a9fb34161..ddc3ac1edb 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -45,7 +45,6 @@ #include "virthread.h" #include "virfile.h" #include "virtime.h" -#include "intprops.h" #include "virstring.h" #include "configmake.h" @@ -832,7 +831,7 @@ virLogNewOutputToSyslog(virLogPriority priority, # define IOVEC_SET_STRING(iov, str) IOVEC_SET(iov, str, strlen(str)) /* Used for conversion of numbers to strings, and for length of binary data */ -# define JOURNAL_BUF_SIZE (MAX(INT_BUFSIZE_BOUND(int), sizeof(uint64_t))) +# define JOURNAL_BUF_SIZE (MAX(VIR_INT64_STR_BUFLEN, sizeof(uint64_t))) struct journalState { diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 3a7a6dc730..a37bcb4004 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -25,7 +25,6 @@ #include "virfile.h" #include "viralloc.h" #include "virlog.h" -#include "intprops.h" #include "virstring.h" #include <sys/ioctl.h> @@ -125,8 +124,7 @@ static int virNetDevBridgeSet(const char *brname, path = g_strdup_printf(SYSFS_NET_DIR "%s/bridge/%s", brname, paramname); if (virFileExists(path)) { - char valuestr[INT_BUFSIZE_BOUND(value)]; - g_snprintf(valuestr, sizeof(valuestr), "%lu", value); + g_autofree char *valuestr = g_strdup_printf("%lu", value); if (virFileWriteStr(path, valuestr, 0) >= 0) return 0; VIR_DEBUG("Unable to set bridge %s %s via sysfs", brname, paramname); @@ -169,7 +167,7 @@ static int virNetDevBridgeGet(const char *brname, if (virFileExists(path)) { g_autofree char *valuestr = NULL; - if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &valuestr) < 0) return -1; @@ -215,7 +213,7 @@ virNetDevBridgePortSet(const char *brname, const char *paramname, unsigned long value) { - char valuestr[INT_BUFSIZE_BOUND(value)]; + char valuestr[VIR_INT64_STR_BUFLEN]; int ret = -1; g_autofree char *path = NULL; @@ -251,7 +249,7 @@ virNetDevBridgePortGet(const char *brname, path = g_strdup_printf(SYSFS_NET_DIR "%s/brif/%s/%s", brname, ifname, paramname); - if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0) + if (virFileReadAll(path, VIR_INT64_STR_BUFLEN, &valuestr) < 0) return -1; if (virStrToLong_ul(valuestr, NULL, 10, value) < 0) { diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index b08e0d8d52..a8a743504d 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -31,7 +31,6 @@ #include "virfile.h" #include "viralloc.h" #include "virutil.h" -#include "intprops.h" #include "virlog.h" #include "virerror.h" #include "virstring.h" @@ -57,7 +56,7 @@ int virPidFileWritePath(const char *pidfile, { int rc; int fd; - char pidstr[INT_BUFSIZE_BOUND(pid)]; + char pidstr[VIR_INT64_STR_BUFLEN]; if ((fd = open(pidfile, O_WRONLY | O_CREAT | O_TRUNC, @@ -110,7 +109,7 @@ int virPidFileReadPath(const char *path, int rc; ssize_t bytes; long long pid_value = 0; - char pidstr[INT_BUFSIZE_BOUND(pid_value)]; + char pidstr[VIR_INT64_STR_BUFLEN]; char *endptr = NULL; *pid = 0; @@ -333,7 +332,7 @@ int virPidFileAcquirePath(const char *path, pid_t pid) { int fd = -1; - char pidstr[INT_BUFSIZE_BOUND(pid)]; + char pidstr[VIR_INT64_STR_BUFLEN]; if (path[0] == '\0') return 0; diff --git a/src/util/virstring.h b/src/util/virstring.h index a2cd92cf83..360c68395c 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -22,6 +22,8 @@ #include "internal.h" +#define VIR_INT64_STR_BUFLEN 21 + char **virStringSplitCount(const char *string, const char *delim, size_t max_tokens, diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 26876850b8..a9b02af7db 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -33,7 +33,6 @@ # include "virlog.h" # include "virmock.h" # include "rpc/virnetsocket.h" -# include "intprops.h" # define VIR_FROM_THIS VIR_FROM_NONE VIR_LOG_INIT("tests.systemdtest"); @@ -548,8 +547,8 @@ testActivation(bool useNames) size_t nsockIP; int ret = -1; size_t i; - char nfdstr[INT_BUFSIZE_BOUND(size_t)]; - char pidstr[INT_BUFSIZE_BOUND(pid_t)]; + char nfdstr[VIR_INT64_STR_BUFLEN]; + char pidstr[VIR_INT64_STR_BUFLEN]; virSystemdActivationMap map[2]; int *fds = NULL; size_t nfds = 0; diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index e357635757..efcdc613c6 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -29,7 +29,6 @@ #include "internal.h" #include "conf/virdomainobjlist.h" -#include "intprops.h" #include "viralloc.h" #include "virmacaddr.h" #include "virxml.h" @@ -1964,7 +1963,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd) bool ret = false; virshDomainListPtr list = NULL; virDomainPtr dom; - char id_buf[INT_BUFSIZE_BOUND(unsigned int)]; + char id_buf[VIR_INT64_STR_BUFLEN]; unsigned int id; unsigned int flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE; vshTablePtr table = NULL; diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c index 7d1e0ccc8a..cf4a249f0a 100644 --- a/tools/virt-login-shell.c +++ b/tools/virt-login-shell.c @@ -33,11 +33,12 @@ * so don't introduce a link time dep, which we must avoid */ #include "gnulib/lib/configmake.h" -#include "gnulib/lib/intprops.h" + +#define VIR_INT64_STR_BUFLEN 21 int main(int argc, char **argv) { - char uidstr[INT_BUFSIZE_BOUND(uid_t)]; - char gidstr[INT_BUFSIZE_BOUND(gid_t)]; + char uidstr[VIR_INT64_STR_BUFLEN]; + char gidstr[VIR_INT64_STR_BUFLEN]; const char * newargv[6]; size_t nargs = 0; char *newenv[] = { -- 2.24.1

The GLib g_size_checked_mul() function is not quite the same signature, and gives compiler warnings due to not correctly casting from gsize to guint64/32. Implementing a replacement for INT_MULTIPLY_OVERFLOW is easy enough to do ourselves. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- build-aux/syntax-check.mk | 15 --------------- src/internal.h | 2 ++ src/libvirt-domain.c | 6 ++---- src/remote/remote_daemon_dispatch.c | 5 ++--- src/remote/remote_driver.c | 5 ++--- 5 files changed, 8 insertions(+), 25 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 7e7c59c3df..6e9328ee63 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1606,20 +1606,6 @@ sc_prohibit_strings_without_use: re='\<(strn?casecmp|ffs(ll)?)\>' \ $(_sc_header_without_use) -# Extract the raw list of symbol names with this: -gl_extract_define_simple = \ - /^\# *define ([A-Z]\w+)\(/ and print $$1 -# Filter out duplicates and convert to a space-separated list: -_intprops_names = \ - $(shell f=$(gnulib_dir)/lib/intprops.h; \ - perl -lne '$(gl_extract_define_simple)' $$f | sort -u | tr '\n' ' ') -# Remove trailing space and convert to a regular expression: -_intprops_syms_re = $(subst $(_sp),|,$(strip $(_intprops_names))) -# Prohibit the inclusion of intprops.h without an actual use. -sc_prohibit_intprops_without_use: - @h='intprops.h' \ - re='\<($(_intprops_syms_re)) *\(' \ - $(_sc_header_without_use) _stddef_syms_re = NULL|offsetof|ptrdiff_t|size_t|wchar_t # Prohibit the inclusion of stddef.h without an actual use. @@ -1714,7 +1700,6 @@ sc_prohibit_defined_have_decl_tests: # ================================================================== gl_other_headers_ ?= \ - intprops.h \ openat.h \ stat-macros.h diff --git a/src/internal.h b/src/internal.h index 686b7cfcc2..e356db6c78 100644 --- a/src/internal.h +++ b/src/internal.h @@ -38,6 +38,8 @@ # define sa_assert(expr) /* empty */ #endif +#define VIR_INT_MULTIPLY_OVERFLOW(a,b) (G_UNLIKELY ((b) > 0 && (a) > G_MAXINT / (b))) + /* The library itself is allowed to use deprecated functions / * variables, so effectively undefine the deprecated attribute * which would otherwise be defined in libvirt.h. diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index d0304e174f..e7585d1872 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -21,8 +21,6 @@ #include <config.h> #include <sys/stat.h> -#include "intprops.h" - #include "datatypes.h" #include "viralloc.h" #include "virfile.h" @@ -7302,7 +7300,7 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps, virCheckPositiveArgGoto(ncpumaps, error); virCheckPositiveArgGoto(maplen, error); - if (INT_MULTIPLY_OVERFLOW(ncpumaps, maplen)) { + if (VIR_INT_MULTIPLY_OVERFLOW(ncpumaps, maplen)) { virReportError(VIR_ERR_OVERFLOW, _("input too large: %d * %d"), ncpumaps, maplen); goto error; @@ -7503,7 +7501,7 @@ virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, else virCheckZeroArgGoto(maplen, error); - if (cpumaps && INT_MULTIPLY_OVERFLOW(maxinfo, maplen)) { + if (cpumaps && VIR_INT_MULTIPLY_OVERFLOW(maxinfo, maplen)) { virReportError(VIR_ERR_OVERFLOW, _("input too large: %d * %d"), maxinfo, maplen); goto error; diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 9c294ddc39..6c00690f68 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -31,7 +31,6 @@ #include "remote_daemon_stream.h" #include "viruuid.h" #include "vircommand.h" -#include "intprops.h" #include "virnetserverservice.h" #include "virnetserver.h" #include "virfile.h" @@ -2755,7 +2754,7 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr server G_GNUC_UNUSED, goto cleanup; } - if (INT_MULTIPLY_OVERFLOW(args->ncpumaps, args->maplen) || + if (VIR_INT_MULTIPLY_OVERFLOW(args->ncpumaps, args->maplen) || args->ncpumaps * args->maplen > REMOTE_CPUMAPS_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen > REMOTE_CPUMAPS_MAX")); goto cleanup; @@ -2898,7 +2897,7 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server G_GNUC_UNUSED, goto cleanup; } - if (INT_MULTIPLY_OVERFLOW(args->maxinfo, args->maplen) || + if (VIR_INT_MULTIPLY_OVERFLOW(args->maxinfo, args->maplen) || args->maxinfo * args->maplen > REMOTE_CPUMAPS_MAX) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen > REMOTE_CPUMAPS_MAX")); goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c11f73ab4d..66472a6cc1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -43,7 +43,6 @@ #include "viralloc.h" #include "virfile.h" #include "vircommand.h" -#include "intprops.h" #include "virtypedparam.h" #include "viruri.h" #include "virauth.h" @@ -2236,7 +2235,7 @@ remoteDomainGetVcpuPinInfo(virDomainPtr domain, goto done; } - if (INT_MULTIPLY_OVERFLOW(ncpumaps, maplen) || + if (VIR_INT_MULTIPLY_OVERFLOW(ncpumaps, maplen) || ncpumaps * maplen > REMOTE_CPUMAPS_MAX) { virReportError(VIR_ERR_RPC, _("vCPU map buffer length exceeds maximum: %d > %d"), @@ -2405,7 +2404,7 @@ remoteDomainGetVcpus(virDomainPtr domain, maxinfo, REMOTE_VCPUINFO_MAX); goto done; } - if (INT_MULTIPLY_OVERFLOW(maxinfo, maplen) || + if (VIR_INT_MULTIPLY_OVERFLOW(maxinfo, maplen) || maxinfo * maplen > REMOTE_CPUMAPS_MAX) { virReportError(VIR_ERR_RPC, _("vCPU map buffer length exceeds maximum: %d > %d"), -- 2.24.1

Some UNIX platforms don't declare 'environ' in their header files. We can unconditionally declare it ourselves to avoid this problem. There is no need to do this in the aa-helper code since that is Linux only code. Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/commandhelper.c | 3 +++ tests/commandtest.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 77cbcd4680..a7a3c44e33 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -31,6 +31,9 @@ #ifndef WIN32 +/* Some UNIX lack it in headers & it doesn't hurt to redeclare */ +extern char **environ; + # define VIR_FROM_THIS VIR_FROM_NONE static int envsort(const void *a, const void *b) diff --git a/tests/commandtest.c b/tests/commandtest.c index 5df1aa4221..cc8676811e 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -58,6 +58,9 @@ main(void) #else +/* Some UNIX lack it in headers & it doesn't hurt to redeclare */ +extern char **environ; + static int checkoutput(const char *testname, char *prefix) { -- 2.24.1

All UNIX platforms we care about have openpty() in the libutil library. Use of pty.h must also be made conditional, excluding Win32. Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- build-aux/syntax-check.mk | 3 +++ configure.ac | 4 ++++ src/util/Makefile.inc.am | 1 + src/util/virfile.c | 14 +++++++++++++- 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 1fd11093d8..687a4ef368 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -2342,3 +2342,6 @@ exclude_file_name_regexp--sc_prohibit_strcmp = \ exclude_file_name_regexp--sc_prohibit_backslash_alignment = \ ^build-aux/syntax-check\.mk$$ + +exclude_file_name_regexp--sc_prohibit_always_true_header_tests = \ + ^src/util/virfile\.c$$ diff --git a/configure.ac b/configure.ac index 8837928358..b28ed4a6dc 100644 --- a/configure.ac +++ b/configure.ac @@ -383,10 +383,13 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([\ ifaddrs.h \ libtasn1.h \ + util.h \ + libutil.h \ linux/magic.h \ mntent.h \ net/ethernet.h \ netinet/tcp.h \ + pty.h \ pwd.h \ stdarg.h \ syslog.h \ @@ -430,6 +433,7 @@ dnl header could be found. AM_CONDITIONAL([HAVE_LIBTASN1], [test "x$ac_cv_header_libtasn1_h" = "xyes"]) AC_CHECK_LIB([intl],[gettext],[]) +AC_CHECK_LIB([util],[openpty],[]) dnl diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index dfa8347853..23de4a6375 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -282,6 +282,7 @@ libvirt_util_la_CFLAGS = \ $(NULL) libvirt_util_la_LIBADD = \ -lm \ + $(OPENPTY_LIBS) \ $(CAPNG_LIBS) \ $(YAJL_LIBS) \ $(LIBNL_LIBS) \ diff --git a/src/util/virfile.c b/src/util/virfile.c index 6eaabf5371..8bd03f8176 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -27,7 +27,19 @@ #include <passfd.h> #include <fcntl.h> -#include <pty.h> +#include <termios.h> +#ifdef HAVE_PTY_H +/* Linux openpty */ +# include <pty.h> +#endif /* !HAVE_PTY_H */ +#ifdef HAVE_UTIL_H +/* macOS openpty */ +# include <util.h> +#endif /* !HAVE_LIBUTIL_H */ +#ifdef HAVE_LIBUTIL_H +/* FreeBSD openpty */ +# include <libutil.h> +#endif /* !HAVE_LIBUTIL_H */ #include <sys/stat.h> #include <sys/types.h> #include <sys/socket.h> -- 2.24.1

We don't need all the platforms gnulib deals with, so this is a cut down version of GNULIB's physmem.c code. This also allows us to integrate libvirt's error reporting functions closer to the error cause. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- build-aux/syntax-check.mk | 2 +- src/conf/capabilities.c | 1 - src/util/virhostcpu.c | 1 - src/util/virhostmem.c | 182 +++++++++++++++++++++++++++++++------- 4 files changed, 153 insertions(+), 33 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 687a4ef368..359688ea54 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -2320,7 +2320,7 @@ exclude_file_name_regexp--sc_prohibit_virXXXFree = \ ^(docs/|tests/|examples/|tools/|build-aux/syntax-check\.mk|src/test/test_driver.c|src/libvirt_public.syms|include/libvirt/libvirt-(domain|network|nodedev|storage|stream|secret|nwfilter|interface|domain-snapshot).h|src/libvirt-(domain|qemu|network|nodedev|storage|stream|secret|nwfilter|interface|domain-snapshot).c$$) exclude_file_name_regexp--sc_prohibit_sysconf_pagesize = \ - ^(build-aux/syntax-check\.mk|src/util/virutil\.c)$$ + ^(build-aux/syntax-check\.mk|src/util/vir(hostmem|util)\.c)$$ exclude_file_name_regexp--sc_prohibit_pthread_create = \ ^(build-aux/syntax-check\.mk|src/util/virthread\.c|tests/.*)$$ diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 9a39858280..ade04b7cd6 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -26,7 +26,6 @@ #include "capabilities.h" #include "cpu_conf.h" #include "domain_conf.h" -#include "physmem.h" #include "storage_conf.h" #include "viralloc.h" #include "virarch.h" diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index f3adc1b4ae..a26e339a0a 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -41,7 +41,6 @@ #include "viralloc.h" #define LIBVIRT_VIRHOSTCPUPRIV_H_ALLOW #include "virhostcpupriv.h" -#include "physmem.h" #include "virerror.h" #include "virarch.h" #include "virfile.h" diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index 34e6f7adcf..9c08b9bd78 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -33,9 +33,13 @@ # include <sys/resource.h> #endif +#ifdef WIN32 +# define WIN32_LEAN_AND_MEAN +# include <windows.h> +#endif + #include "viralloc.h" #include "virhostmem.h" -#include "physmem.h" #include "virerror.h" #include "virarch.h" #include "virfile.h" @@ -577,13 +581,151 @@ virHostMemGetParameters(virTypedParameterPtr params G_GNUC_UNUSED, } +#ifdef WIN32 +/* MEMORYSTATUSEX is missing from older windows headers, so define + a local replacement. */ +typedef struct +{ + DWORD dwLength; + DWORD dwMemoryLoad; + DWORDLONG ullTotalPhys; + DWORDLONG ullAvailPhys; + DWORDLONG ullTotalPageFile; + DWORDLONG ullAvailPageFile; + DWORDLONG ullTotalVirtual; + DWORDLONG ullAvailVirtual; + DWORDLONG ullAvailExtendedVirtual; +} lMEMORYSTATUSEX; +typedef WINBOOL(WINAPI *PFN_MS_EX) (lMEMORYSTATUSEX*); +#endif /* !WIN32 */ + +static unsigned long long +virHostMemGetTotal(void) +{ +#if defined HAVE_SYSCTLBYNAME + /* This works on freebsd & macOS. */ + unsigned long long physmem = 0; + size_t len = sizeof(physmem); + + if (sysctlbyname("hw.physmem", &physmem, &len, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory total")); + return 0; + } + + return physmem; +#elif defined _SC_PHYS_PAGES && defined _SC_PAGESIZE + /* this works on linux */ + long long pages; + long long pagesize; + if ((pages = sysconf(_SC_PHYS_PAGES)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory total")); + return 0; + } + if ((pagesize = sysconf(_SC_PAGESIZE)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory page size")); + return 0; + } + return (unsigned long long)pages * (unsigned long long)pagesize; +#elif defined WIN32 + PFN_MS_EX pfnex; + HMODULE h = GetModuleHandle("kernel32.dll"); + + if (!h) { + virReportSystemError(errno, "%s", + _("Unable to access kernel32.dll")); + return 0; + } + + /* Use GlobalMemoryStatusEx if available. */ + if ((pfnex = (PFN_MS_EX) GetProcAddress(h, "GlobalMemoryStatusEx"))) { + lMEMORYSTATUSEX lms_ex; + lms_ex.dwLength = sizeof(lms_ex); + if (!pfnex(&lms_ex)) { + virReportSystemError(EIO, "%s", + _("Unable to query memory total")); + return 0; + } + return lms_ex.ullTotalPhys; + } else { + /* Fall back to GlobalMemoryStatus which is always available. + but returns wrong results for physical memory > 4GB. */ + MEMORYSTATUS ms; + GlobalMemoryStatus(&ms); + return ms.dwTotalPhys; + } +#endif +} + + +static unsigned long long +virHostMemGetAvailable(void) +{ +#if defined HAVE_SYSCTLBYNAME + /* This works on freebsd and macOS */ + unsigned long long usermem = 0; + size_t len = sizeof(usermem); + + if (sysctlbyname("hw.usermem", &usermem, &len, NULL, 0) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory available")); + return 0; + } + + return usermem; +#elif defined _SC_AVPHYS_PAGES && defined _SC_PAGESIZE + /* this works on linux */ + long long pages; + long long pagesize; + if ((pages = sysconf(_SC_AVPHYS_PAGES)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory available")); + return 0; + } + if ((pagesize = sysconf(_SC_PAGESIZE)) < 0) { + virReportSystemError(errno, "%s", + _("Unable to query memory page size")); + return 0; + } + return (unsigned long long)pages * (unsigned long long)pagesize; +#elif defined WIN32 + PFN_MS_EX pfnex; + HMODULE h = GetModuleHandle("kernel32.dll"); + + if (!h) { + virReportSystemError(errno, "%s", + _("Unable to access kernel32.dll")); + return 0; + } + + /* Use GlobalMemoryStatusEx if available. */ + if ((pfnex = (PFN_MS_EX) GetProcAddress(h, "GlobalMemoryStatusEx"))) { + lMEMORYSTATUSEX lms_ex; + lms_ex.dwLength = sizeof(lms_ex); + if (!pfnex(&lms_ex)) { + virReportSystemError(EIO, "%s", + _("Unable to query memory available")); + return 0; + } + return lms_ex.ullAvailPhys; + } else { + /* Fall back to GlobalMemoryStatus which is always available. + but returns wrong results for physical memory > 4GB */ + MEMORYSTATUS ms; + GlobalMemoryStatus(&ms); + return ms.dwAvailPhys; + } +#endif +} + + static int virHostMemGetCellsFreeFake(unsigned long long *freeMems, int startCell, int maxCells G_GNUC_UNUSED) { - double avail = physmem_available(); - if (startCell != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("start cell %d out of range (0-%d)"), @@ -591,13 +733,8 @@ virHostMemGetCellsFreeFake(unsigned long long *freeMems, return -1; } - freeMems[0] = (unsigned long long)avail; - - if (!freeMems[0]) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot determine free memory")); + if ((freeMems[0] = virHostMemGetAvailable()) == 0) return -1; - } return 1; } @@ -606,28 +743,13 @@ static int virHostMemGetInfoFake(unsigned long long *mem, unsigned long long *freeMem) { - if (mem) { - double total = physmem_total(); - if (!total) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot determine free memory")); - return -1; - } - - *mem = (unsigned long long) total; - } - - if (freeMem) { - double avail = physmem_available(); - - if (!avail) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot determine free memory")); - return -1; - } + if (mem && + (*mem = virHostMemGetTotal()) == 0) + return -1; - *freeMem = (unsigned long long) avail; - } + if (freeMem && + (*freeMem = virHostMemGetAvailable()) == 0) + return -1; return 0; } -- 2.24.1

Libvirt's original atomic ops impls were largely copied from GLib's code at the time. The only API difference was that libvirt's virAtomicIntInc() would return a value, but g_atomic_int_inc was void. We thus use g_atomic_int_add(v, 1) instead, though this means virAtomicIntInc() now returns the original value, instead of the new value. This rewrites libvirt's impl in terms of g_atomic_int* as a short term conversion. The key motivation was to quickly eliminate use of GNULIB's verify_expr() macro which is not a direct match for G_STATIC_ASSERT_EXPR. Long term all the callers should be updated to use g_atomic_int* directly. Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/libxl/libxl_domain.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_process.c | 4 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- src/qemu/qemu_process.c | 4 +- src/util/viratomic.h | 351 ++---------------------------- src/util/virprocess.c | 2 +- tests/viratomictest.c | 2 +- 8 files changed, 26 insertions(+), 347 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 915aaeb8b0..f9be4ad583 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1473,7 +1473,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) goto destroy_dom; - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); /* finally we can call the 'started' hook script if any */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index f021ec9c5d..ef02a066d9 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -446,7 +446,7 @@ libxlReconnectDomain(virDomainObjPtr vm, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN); - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); /* Enable domain death events */ diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0a9ccdf9ec..af8593d6a5 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1468,7 +1468,7 @@ int virLXCProcessStart(virConnectPtr conn, if (virCommandHandshakeNotify(cmd) < 0) goto cleanup; - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); if (lxcContainerWaitForContinue(handshakefds[0]) < 0) { @@ -1670,7 +1670,7 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm, virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN); - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 9a71d13d57..f3acaf00dd 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -888,7 +888,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, skip_instantiate: VIR_FREE(ipl); - virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases); + ignore_value(virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases)); lease_not_found: VIR_FREE(ipstr); @@ -1167,7 +1167,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) _("Instantiation of rules failed on " "interface '%s'"), req->binding->portdevname); } - virAtomicIntDecAndTest(job->qCtr); + ignore_value(virAtomicIntDecAndTest(job->qCtr)); VIR_FREE(job); } @@ -1568,7 +1568,7 @@ virNWFilterDHCPSnoopThread(void *req0) pcap_close(pcapConf[i].handle); } - virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads); + ignore_value(virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads)); return; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a7bbab9e56..420d1c9c93 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5571,7 +5571,7 @@ qemuProcessInit(virQEMUDriverPtr driver, qemuDomainSetFakeReboot(driver, vm, false); virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP); - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); /* Run an early hook to set-up missing devices */ @@ -8139,7 +8139,7 @@ qemuProcessReconnect(void *opaque) goto error; } - if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) + if (virAtomicIntInc(&driver->nactive) == 0 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); cleanup: diff --git a/src/util/viratomic.h b/src/util/viratomic.h index 9dfb77b992..12b116a6cd 100644 --- a/src/util/viratomic.h +++ b/src/util/viratomic.h @@ -1,11 +1,7 @@ /* * viratomic.h: atomic integer operations * - * Copyright (C) 2012 Red Hat, Inc. - * - * Based on code taken from GLib 2.32, under the LGPLv2+ - * - * Copyright (C) 2011 Ryan Lortie + * Copyright (C) 2012-2020 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -21,18 +17,15 @@ * License along with this library. If not, see * <http://www.gnu.org/licenses/>. * + * APIs in this header should no longer be used. Direct + * use of the g_atomic APIs is preferred & existing code + * should be converted as needed. */ #pragma once #include "internal.h" -#ifdef VIR_ATOMIC_OPS_GCC -# define VIR_STATIC /* Nothing; we just never define the functions */ -#else -# define VIR_STATIC static -#endif - /** * virAtomicIntGet: * Gets the current value of atomic. @@ -40,8 +33,7 @@ * This call acts as a full compiler and hardware memory barrier * (before the get) */ -VIR_STATIC int virAtomicIntGet(volatile int *atomic) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntGet(v) g_atomic_int_get(v) /** * virAtomicIntSet: @@ -50,21 +42,18 @@ VIR_STATIC int virAtomicIntGet(volatile int *atomic) * This call acts as a full compiler and hardware memory barrier * (after the set) */ -VIR_STATIC void virAtomicIntSet(volatile int *atomic, - int newval) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntSet(i, newv) g_atomic_int_set(i, newv) /** * virAtomicIntInc: * Increments the value of atomic by 1. * * Think of this operation as an atomic version of - * { *atomic += 1; return *atomic; } + * { tmp = *atomic; *atomic += 1; return tmp; } * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC int virAtomicIntInc(volatile int *atomic) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntInc(i) g_atomic_int_add(i, 1) /** * virAtomicIntDecAndTest: @@ -75,8 +64,7 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic) * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntDecAndTest(i) (!!g_atomic_int_dec_and_test(i)) /** * virAtomicIntCompareExchange: @@ -91,10 +79,8 @@ VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic) * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC bool virAtomicIntCompareExchange(volatile int *atomic, - int oldval, - int newval) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntCompareExchange(i, oldi, newi) \ + (!!g_atomic_int_compare_and_exchange(i, oldi, newi)) /** * virAtomicIntAdd: @@ -105,9 +91,7 @@ VIR_STATIC bool virAtomicIntCompareExchange(volatile int *atomic, * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC int virAtomicIntAdd(volatile int *atomic, - int val) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntAdd(i, v) g_atomic_int_add(i, v) /** * virAtomicIntAnd: @@ -119,9 +103,7 @@ VIR_STATIC int virAtomicIntAdd(volatile int *atomic, * Think of this operation as an atomic version of * { tmp = *atomic; *atomic &= val; return tmp; } */ -VIR_STATIC unsigned int virAtomicIntAnd(volatile unsigned int *atomic, - unsigned int val) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntAnd(i, v) g_atomic_int_and(i, v) /** * virAtomicIntOr: @@ -133,9 +115,7 @@ VIR_STATIC unsigned int virAtomicIntAnd(volatile unsigned int *atomic, * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC unsigned int virAtomicIntOr(volatile unsigned int *atomic, - unsigned int val) - ATTRIBUTE_NONNULL(1); +#define virAtomicIntOr(i, v) g_atomic_int_or(i, v) /** * virAtomicIntXor: @@ -147,305 +127,4 @@ VIR_STATIC unsigned int virAtomicIntOr(volatile unsigned int *atomic, * * This call acts as a full compiler and hardware memory barrier. */ -VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic, - unsigned int val) - ATTRIBUTE_NONNULL(1); - -#undef VIR_STATIC - -#ifdef VIR_ATOMIC_OPS_GCC - -# define virAtomicIntGet(atomic) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ *(atomic) : 0); \ - __sync_synchronize(); \ - (int)*(atomic); \ - })) -# define virAtomicIntSet(atomic, newval) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ (newval) : 0); \ - *(atomic) = (newval); \ - __sync_synchronize(); \ - })) -# define virAtomicIntInc(atomic) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ *(atomic) : 0); \ - __sync_add_and_fetch((atomic), 1); \ - })) -# define virAtomicIntDecAndTest(atomic) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ *(atomic) : 0); \ - __sync_fetch_and_sub((atomic), 1) == 1; \ - })) -# define virAtomicIntCompareExchange(atomic, oldval, newval) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ (newval) ^ (oldval) : 0); \ - (bool)__sync_bool_compare_and_swap((atomic), \ - (oldval), (newval)); \ - })) -# define virAtomicIntAdd(atomic, val) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void)(0 ? *(atomic) ^ (val) : 0); \ - (int) __sync_fetch_and_add((atomic), (val)); \ - })) -# define virAtomicIntAnd(atomic, val) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void) (0 ? *(atomic) ^ (val) : 0); \ - (unsigned int) __sync_fetch_and_and((atomic), (val)); \ - })) -# define virAtomicIntOr(atomic, val) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void) (0 ? *(atomic) ^ (val) : 0); \ - (unsigned int) __sync_fetch_and_or((atomic), (val)); \ - })) -# define virAtomicIntXor(atomic, val) \ - (__extension__ ({ \ - (void)verify_expr(sizeof(*(atomic)) == sizeof(int), 0); \ - (void) (0 ? *(atomic) ^ (val) : 0); \ - (unsigned int) __sync_fetch_and_xor((atomic), (val)); \ - })) - - -#else - -# ifdef VIR_ATOMIC_OPS_WIN32 - -# include <winsock2.h> -# include <windows.h> -# include <intrin.h> -# if !defined(_M_AMD64) && !defined (_M_IA64) && !defined(_M_X64) -# define InterlockedAnd _InterlockedAnd -# define InterlockedOr _InterlockedOr -# define InterlockedXor _InterlockedXor -# endif - -/* - * http://msdn.microsoft.com/en-us/library/ms684122(v=vs.85).aspx - */ -static inline int -virAtomicIntGet(volatile int *atomic) -{ - MemoryBarrier(); - return *atomic; -} - -static inline void -virAtomicIntSet(volatile int *atomic, - int newval) -{ - *atomic = newval; - MemoryBarrier(); -} - -static inline int -virAtomicIntInc(volatile int *atomic) -{ - return InterlockedIncrement((volatile LONG *)atomic); -} - -static inline bool -virAtomicIntDecAndTest(volatile int *atomic) -{ - return InterlockedDecrement((volatile LONG *)atomic) == 0; -} - -static inline bool -virAtomicIntCompareExchange(volatile int *atomic, - int oldval, - int newval) -{ - return InterlockedCompareExchange((volatile LONG *)atomic, newval, oldval) == oldval; -} - -static inline int -virAtomicIntAdd(volatile int *atomic, - int val) -{ - return InterlockedExchangeAdd((volatile LONG *)atomic, val); -} - -static inline unsigned int -virAtomicIntAnd(volatile unsigned int *atomic, - unsigned int val) -{ - return InterlockedAnd((volatile LONG *)atomic, val); -} - -static inline unsigned int -virAtomicIntOr(volatile unsigned int *atomic, - unsigned int val) -{ - return InterlockedOr((volatile LONG *)atomic, val); -} - -static inline unsigned int -virAtomicIntXor(volatile unsigned int *atomic, - unsigned int val) -{ - return InterlockedXor((volatile LONG *)atomic, val); -} - - -# else -# ifdef VIR_ATOMIC_OPS_PTHREAD -# include <pthread.h> - -extern pthread_mutex_t virAtomicLock; - -static inline int -virAtomicIntGet(volatile int *atomic) -{ - int value; - - pthread_mutex_lock(&virAtomicLock); - value = *atomic; - pthread_mutex_unlock(&virAtomicLock); - - return value; -} - -static inline void -virAtomicIntSet(volatile int *atomic, - int value) -{ - pthread_mutex_lock(&virAtomicLock); - *atomic = value; - pthread_mutex_unlock(&virAtomicLock); -} - -static inline int -virAtomicIntInc(volatile int *atomic) -{ - int value; - - pthread_mutex_lock(&virAtomicLock); - value = ++(*atomic); - pthread_mutex_unlock(&virAtomicLock); - - return value; -} - -static inline bool -virAtomicIntDecAndTest(volatile int *atomic) -{ - bool is_zero; - - pthread_mutex_lock(&virAtomicLock); - is_zero = --(*atomic) == 0; - pthread_mutex_unlock(&virAtomicLock); - - return is_zero; -} - -static inline bool -virAtomicIntCompareExchange(volatile int *atomic, - int oldval, - int newval) -{ - bool success; - - pthread_mutex_lock(&virAtomicLock); - - if ((success = (*atomic == oldval))) - *atomic = newval; - - pthread_mutex_unlock(&virAtomicLock); - - return success; -} - -static inline int -virAtomicIntAdd(volatile int *atomic, - int val) -{ - int oldval; - - pthread_mutex_lock(&virAtomicLock); - oldval = *atomic; - *atomic = oldval + val; - pthread_mutex_unlock(&virAtomicLock); - - return oldval; -} - -static inline unsigned int -virAtomicIntAnd(volatile unsigned int *atomic, - unsigned int val) -{ - unsigned int oldval; - - pthread_mutex_lock(&virAtomicLock); - oldval = *atomic; - *atomic = oldval & val; - pthread_mutex_unlock(&virAtomicLock); - - return oldval; -} - -static inline unsigned int -virAtomicIntOr(volatile unsigned int *atomic, - unsigned int val) -{ - unsigned int oldval; - - pthread_mutex_lock(&virAtomicLock); - oldval = *atomic; - *atomic = oldval | val; - pthread_mutex_unlock(&virAtomicLock); - - return oldval; -} - -static inline unsigned int -virAtomicIntXor(volatile unsigned int *atomic, - unsigned int val) -{ - unsigned int oldval; - - pthread_mutex_lock(&virAtomicLock); - oldval = *atomic; - *atomic = oldval ^ val; - pthread_mutex_unlock(&virAtomicLock); - - return oldval; -} - - -# else -# error "No atomic integer impl for this platform" -# endif -# endif - -/* The int/unsigned int casts here ensure that you can - * pass either an int or unsigned int to all atomic op - * functions, in the same way that we can with GCC - * atomic op helpers. - */ -# define virAtomicIntGet(atomic) \ - virAtomicIntGet((int *)atomic) -# define virAtomicIntSet(atomic, val) \ - virAtomicIntSet((int *)atomic, val) -# define virAtomicIntInc(atomic) \ - virAtomicIntInc((int *)atomic) -# define virAtomicIntDecAndTest(atomic) \ - virAtomicIntDecAndTest((int *)atomic) -# define virAtomicIntCompareExchange(atomic, oldval, newval) \ - virAtomicIntCompareExchange((int *)atomic, oldval, newval) -# define virAtomicIntAdd(atomic, val) \ - virAtomicIntAdd((int *)atomic, val) -# define virAtomicIntAnd(atomic, val) \ - virAtomicIntAnd((unsigned int *)atomic, val) -# define virAtomicIntOr(atomic, val) \ - virAtomicIntOr((unsigned int *)atomic, val) -# define virAtomicIntXor(atomic, val) \ - virAtomicIntXor((unsigned int *)atomic, val) - -#endif +#define virAtomicIntXor(i, v) g_atomic_int_xor(i, v) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 32f19e6b63..d5589daf6a 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1021,7 +1021,7 @@ int virProcessGetStartTime(pid_t pid, unsigned long long *timestamp) { static int warned; - if (virAtomicIntInc(&warned) == 1) { + if (virAtomicIntInc(&warned) == 0) { VIR_WARN("Process start time of pid %lld not available on this platform", (long long) pid); } diff --git a/tests/viratomictest.c b/tests/viratomictest.c index 8c885a5b96..e30df66cd7 100644 --- a/tests/viratomictest.c +++ b/tests/viratomictest.c @@ -50,7 +50,7 @@ testTypes(const void *data G_GNUC_UNUSED) testAssertEq(virAtomicIntAdd(&u, 1), 5); testAssertEq(u, 6); - testAssertEq(virAtomicIntInc(&u), 7); + testAssertEq(virAtomicIntInc(&u), 6); testAssertEq(u, 7); res = virAtomicIntDecAndTest(&u); -- 2.24.1

The GNULIB termios module ensures termios.h exists (but is none the less empty) when building for Windows. We already exclude usage of the functions that would exist in a real termios.h, so having an empty termios.h is not especially useful. It is simpler to just put all use of termios.h related functions behind a "#ifndef WIN32" conditional. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- configure.ac | 1 - src/util/virfdstream.c | 10 ++++++---- src/util/virfile.c | 4 +++- src/util/virutil.c | 1 - tools/virsh.h | 1 - tools/vsh.c | 15 --------------- tools/vsh.h | 4 +++- 7 files changed, 12 insertions(+), 24 deletions(-) diff --git a/configure.ac b/configure.ac index b28ed4a6dc..7d69b8bcb4 100644 --- a/configure.ac +++ b/configure.ac @@ -353,7 +353,6 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([\ - cfmakeraw \ fallocate \ geteuid \ getgid \ diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c index 719185d992..a903107afb 100644 --- a/src/util/virfdstream.c +++ b/src/util/virfdstream.c @@ -31,7 +31,9 @@ # include <sys/un.h> #endif #include <netinet/in.h> -#include <termios.h> +#ifndef WIN32 +# include <termios.h> +#endif #include "virfdstream.h" #include "virerror.h" @@ -1361,7 +1363,7 @@ int virFDStreamCreateFile(virStreamPtr st, false, false); } -#ifdef HAVE_CFMAKERAW +#ifndef WIN32 int virFDStreamOpenPTY(virStreamPtr st, const char *path, unsigned long long offset, @@ -1401,7 +1403,7 @@ int virFDStreamOpenPTY(virStreamPtr st, virFDStreamClose(st); return -1; } -#else /* !HAVE_CFMAKERAW */ +#else /* WIN32 */ int virFDStreamOpenPTY(virStreamPtr st, const char *path, unsigned long long offset, @@ -1413,7 +1415,7 @@ int virFDStreamOpenPTY(virStreamPtr st, oflags | O_CREAT, 0, false, false); } -#endif /* !HAVE_CFMAKERAW */ +#endif /* WIN32 */ int virFDStreamOpenBlockDevice(virStreamPtr st, const char *path, diff --git a/src/util/virfile.c b/src/util/virfile.c index 8bd03f8176..b3a63fa2ea 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -27,7 +27,9 @@ #include <passfd.h> #include <fcntl.h> -#include <termios.h> +#ifndef WIN32 +# include <termios.h> +#endif /* !WIN32 */ #ifdef HAVE_PTY_H /* Linux openpty */ # include <pty.h> diff --git a/src/util/virutil.c b/src/util/virutil.c index c14c02a8c4..7c2c5a78f6 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -35,7 +35,6 @@ #endif #include <sys/types.h> -#include <termios.h> #if WITH_DEVMAPPER # include <libdevmapper.h> diff --git a/tools/virsh.h b/tools/virsh.h index 903a2e53b6..fa9e54b1d1 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -23,7 +23,6 @@ #include <stdarg.h> #include <unistd.h> #include <sys/stat.h> -#include <termios.h> #include "internal.h" #include "virerror.h" diff --git a/tools/vsh.c b/tools/vsh.c index a36b6bfe23..5c8908f240 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1928,21 +1928,6 @@ vshTTYRestore(vshControl *ctl G_GNUC_UNUSED) } -#if !defined(WIN32) && !defined(HAVE_CFMAKERAW) -/* provide fallback in case cfmakeraw isn't available */ -static void -cfmakeraw(struct termios *attr) -{ - attr->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP - | INLCR | IGNCR | ICRNL | IXON); - attr->c_oflag &= ~OPOST; - attr->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); - attr->c_cflag &= ~(CSIZE | PARENB); - attr->c_cflag |= CS8; -} -#endif /* !WIN32 && !HAVE_CFMAKERAW */ - - int vshTTYMakeRaw(vshControl *ctl G_GNUC_UNUSED, bool report_errors G_GNUC_UNUSED) diff --git a/tools/vsh.h b/tools/vsh.h index 960cae8df0..174116b369 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -23,7 +23,9 @@ #include <stdarg.h> #include <unistd.h> #include <sys/stat.h> -#include <termios.h> +#ifndef WIN32 +# include <termios.h> +#endif #include "internal.h" #include "virerror.h" -- 2.24.1

gmtime_r/localtime_r are mostly used in combination with strftime to format timestamps in libvirt. This can all be replaced with GDateTime resulting in simpler code that is also more portable. There is some boundary condition problem in parsing POSIX timezone offsets in GLib which tickles our test suite. The test suite is hacked to avoid the problem. The upsteam GLib bug report is https://gitlab.gnome.org/GNOME/glib/issues/1999 Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 11 ++++---- src/libxl/libxl_domain.c | 10 +++---- src/qemu/qemu_command.c | 17 ++++------- src/qemu/qemu_driver.c | 10 +++---- src/util/virtime.c | 35 ++++------------------- tests/qemuxml2argvmock.c | 12 ++++---- tests/virtimetest.c | 39 +++++++++++++------------ tools/virsh-checkpoint.c | 20 +++++-------- tools/virsh-domain-monitor.c | 14 ++++----- tools/virsh-domain.c | 13 ++++----- tools/virsh-network.c | 11 ++++---- tools/virsh-snapshot.c | 19 ++++--------- tools/virt-admin.c | 55 ++++++++---------------------------- tools/vsh.c | 16 ++++------- 14 files changed, 96 insertions(+), 186 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a8a04cacb..91ddad4d86 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26789,11 +26789,12 @@ virDomainGraphicsAuthDefFormatAttr(virBufferPtr buf, def->passwd); if (def->expires) { - char strbuf[100]; - struct tm tmbuf, *tm; - tm = gmtime_r(&def->validTo, &tmbuf); - strftime(strbuf, sizeof(strbuf), "%Y-%m-%dT%H:%M:%S", tm); - virBufferAsprintf(buf, " passwdValidTo='%s'", strbuf); + g_autoptr(GDateTime) then = NULL; + g_autofree char *thenstr = NULL; + + then = g_date_time_new_from_unix_utc(def->validTo); + thenstr = g_date_time_format(then, "%Y-%m-%dT%H:%M:%S"); + virBufferAsprintf(buf, " passwdValidTo='%s'", thenstr); } if (def->connected) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index f9be4ad583..d63eca0bd6 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -943,16 +943,14 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver, virDomainObjPtr vm) { g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); - time_t curtime = time(NULL); - char timestr[100]; - struct tm time_info; + g_autoptr(GDateTime) now = g_date_time_new_now_local(); + g_autofree char *nowstr = NULL; char *dumpfile = NULL; - localtime_r(&curtime, &time_info); - strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); + nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S"); dumpfile = g_strdup_printf("%s/%s-%s", cfg->autoDumpDir, vm->def->name, - timestr); + nowstr); /* Unlock virDomainObj while dumping core */ virObjectUnlock(vm); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 904d2beab5..62cd3be7c4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6070,8 +6070,9 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) break; case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: { - time_t now = time(NULL); - struct tm nowbits; + g_autoptr(GDateTime) now = g_date_time_new_now_utc(); + g_autoptr(GDateTime) then = NULL; + g_autofree char *thenstr = NULL; if (def->data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) { long localOffset; @@ -6094,8 +6095,8 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) def->data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC; } - now += def->data.variable.adjustment; - gmtime_r(&now, &nowbits); + then = g_date_time_add_seconds(now, def->data.variable.adjustment); + thenstr = g_date_time_format(then, "%Y-%m-%dT%H:%M:%S"); /* when an RTC_CHANGE event is received from qemu, we need to * have the adjustment used at domain start time available to @@ -6105,13 +6106,7 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) */ def->data.variable.adjustment0 = def->data.variable.adjustment; - virBufferAsprintf(&buf, "base=%d-%02d-%02dT%02d:%02d:%02d", - nowbits.tm_year + 1900, - nowbits.tm_mon + 1, - nowbits.tm_mday, - nowbits.tm_hour, - nowbits.tm_min, - nowbits.tm_sec); + virBufferAsprintf(&buf, "base=%s", thenstr); } break; default: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4862d75923..b73079001a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4090,9 +4090,8 @@ getAutoDumpPath(virQEMUDriverPtr driver, virDomainObjPtr vm) { g_autofree char *domname = virDomainDefGetShortName(vm->def); - char timestr[100]; - struct tm time_info; - time_t curtime = time(NULL); + g_autoptr(GDateTime) now = g_date_time_new_now_local(); + g_autofree char *nowstr = NULL; g_autoptr(virQEMUDriverConfig) cfg = NULL; if (!domname) @@ -4100,10 +4099,9 @@ getAutoDumpPath(virQEMUDriverPtr driver, cfg = virQEMUDriverGetConfig(driver); - localtime_r(&curtime, &time_info); - strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); + nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S"); - return g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, timestr); + return g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, nowstr); } static void diff --git a/src/util/virtime.c b/src/util/virtime.c index 13f899cb91..bc8f06cd48 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -302,8 +302,8 @@ char *virTimeStringThen(unsigned long long when) /** * virTimeLocalOffsetFromUTC: * - * This function is threadsafe, but is *not* async signal safe (due to - * gmtime_r() and mktime()). + * This function is threadsafe, but is *not* async signal safe + * due to use of GLib APIs. * * @offset: pointer to time_t that will be set to the difference * between localtime and UTC in seconds (east of UTC is a @@ -314,34 +314,11 @@ char *virTimeStringThen(unsigned long long when) int virTimeLocalOffsetFromUTC(long *offset) { - struct tm gmtimeinfo; - time_t current, utc; + g_autoptr(GDateTime) now = g_date_time_new_now_local(); + GTimeSpan diff = g_date_time_get_utc_offset(now); - /* time() gives seconds since Epoch in current timezone */ - if ((current = time(NULL)) == (time_t)-1) { - virReportSystemError(errno, "%s", - _("failed to get current system time")); - return -1; - } - - /* treat current as if it were in UTC */ - if (!gmtime_r(¤t, &gmtimeinfo)) { - virReportSystemError(errno, "%s", - _("gmtime_r failed")); - return -1; - } - - /* tell mktime to figure out itself whether or not DST is in effect */ - gmtimeinfo.tm_isdst = -1; - - /* mktime() also obeys current timezone rules */ - if ((utc = mktime(&gmtimeinfo)) == (time_t)-1) { - virReportSystemError(errno, "%s", - _("mktime failed")); - return -1; - } - - *offset = current - utc; + /* GTimeSpan measures microseconds, we want seconds */ + *offset = diff / 1000000; return 0; } diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index 8143de1618..e5841bc8e3 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -47,12 +47,14 @@ long virGetSystemPageSize(void) return 4096; } -time_t time(time_t *t) +GDateTime *g_date_time_new_now_utc(void) { - const time_t ret = 1234567890; - if (t) - *t = ret; - return ret; + return g_date_time_new_from_unix_utc(1234567890); +} + +GDateTime *g_date_time_new_now_local(void) +{ + return g_date_time_new_from_unix_local(1234567890); } bool diff --git a/tests/virtimetest.c b/tests/virtimetest.c index f8a81ff090..f9ac55192d 100644 --- a/tests/virtimetest.c +++ b/tests/virtimetest.c @@ -101,20 +101,12 @@ testTimeLocalOffset(const void *args) static bool isNearYearEnd(void) { - time_t current = time(NULL); - struct tm timeinfo; + g_autoptr(GDateTime) now = g_date_time_new_now_local(); - if (current == (time_t)-1) { - VIR_DEBUG("time() failed"); - return false; - } - if (!localtime_r(¤t, &timeinfo)) { - VIR_DEBUG("localtime_r() failed"); - return false; - } - - return (timeinfo.tm_mon == 0 && timeinfo.tm_mday == 1) || - (timeinfo.tm_mon == 11 && timeinfo.tm_mday == 31); + return ((g_date_time_get_month(now) == 1 && + g_date_time_get_day_of_month(now) == 1) || + (g_date_time_get_month(now) == 12 && + g_date_time_get_day_of_month(now) == 31)); } @@ -186,14 +178,21 @@ mymain(void) /* test DST processing with timezones that always * have DST in effect; what's more, cover a zone with * with an unusual DST different than a usual one hour + * + * These tests originally used '0' as the first day, + * but changed to '1' due to GLib GTimeZone parsing bug: + * https://gitlab.gnome.org/GNOME/glib/issues/1999 + * + * Once we depend on a new enough GLib, we can put then + * back to 0 again. */ - TEST_LOCALOFFSET("VIR-00:30VID,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR-00:30VID,1/00:00:00,364/23:59:59", ((1 * 60) + 30) * 60); - TEST_LOCALOFFSET("VIR-02:30VID,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR-02:30VID,1/00:00:00,364/23:59:59", ((3 * 60) + 30) * 60); - TEST_LOCALOFFSET("VIR-02:30VID-04:30,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR-02:30VID-04:30,1/00:00:00,364/23:59:59", ((4 * 60) + 30) * 60); - TEST_LOCALOFFSET("VIR-12:00VID-13:00,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR-12:00VID-13:00,1/00:00:00,364/23:59:59", ((13 * 60) + 0) * 60); if (!isNearYearEnd()) { @@ -209,11 +208,11 @@ mymain(void) * tests, except on Dec 31 and Jan 1. */ - TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR02:45VID00:45,1/00:00:00,364/23:59:59", -45 * 60); - TEST_LOCALOFFSET("VIR05:00VID04:00,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR05:00VID04:00,1/00:00:00,364/23:59:59", ((-4 * 60) + 0) * 60); - TEST_LOCALOFFSET("VIR11:00VID10:00,0/00:00:00,365/23:59:59", + TEST_LOCALOFFSET("VIR11:00VID10:00,1/00:00:00,364/23:59:59", ((-10 * 60) + 0) * 60); } diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index f9749b5f6d..e82a67f075 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -719,9 +719,8 @@ cmdCheckpointList(vshControl *ctl, char *doc = NULL; virDomainCheckpointPtr checkpoint = NULL; long long creation_longlong; - time_t creation_time_t; - char timestr[100]; - struct tm time_info; + g_autoptr(GDateTime) then = NULL; + g_autofree gchar *thenstr = NULL; bool tree = vshCommandOptBool(cmd, "tree"); bool name = vshCommandOptBool(cmd, "name"); bool from = vshCommandOptBool(cmd, "from"); @@ -835,21 +834,16 @@ cmdCheckpointList(vshControl *ctl, if (virXPathLongLong("string(/domaincheckpoint/creationTime)", ctxt, &creation_longlong) < 0) continue; - creation_time_t = creation_longlong; - if (creation_time_t != creation_longlong) { - vshError(ctl, "%s", _("time_t overflow")); - continue; - } - localtime_r(&creation_time_t, &time_info); - strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z", - &time_info); + + then = g_date_time_new_from_unix_local(creation_longlong); + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S %z"); if (parent) { - if (vshTableRowAppend(table, chk_name, timestr, + if (vshTableRowAppend(table, chk_name, thenstr, NULLSTR_EMPTY(parent_chk), NULL) < 0) goto cleanup; } else { - if (vshTableRowAppend(table, chk_name, timestr, NULL) < 0) + if (vshTableRowAppend(table, chk_name, thenstr, NULL) < 0) goto cleanup; } } diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index efcdc613c6..933e2aa75f 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1534,17 +1534,13 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (pretty) { - char timestr[100]; - time_t cur_time = seconds; - struct tm time_info; + g_autoptr(GDateTime) then = NULL; + g_autofree char *thenstr = NULL; - if (!gmtime_r(&cur_time, &time_info)) { - vshError(ctl, _("Unable to format time")); - goto cleanup; - } - strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info); + then = g_date_time_new_from_unix_utc(seconds); + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S"); - vshPrint(ctl, _("Time: %s"), timestr); + vshPrint(ctl, _("Time: %s"), thenstr); } else { vshPrint(ctl, _("Time: %lld"), seconds); } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c48575279c..e64e08e5da 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5492,9 +5492,8 @@ static const vshCmdOptDef opts_screenshot[] = { static char * virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) { - char timestr[100]; - time_t cur_time; - struct tm time_info; + g_autoptr(GDateTime) now = g_date_time_new_now_local(); + g_autofree char *nowstr = NULL; const char *ext = NULL; char *ret = NULL; @@ -5509,12 +5508,10 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime) ext = ".png"; /* add mime type here */ - time(&cur_time); - localtime_r(&cur_time, &time_info); - strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info); + nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S"); - ret = g_strdup_printf("%s-%s%s", virDomainGetName(dom), timestr, - NULLSTR_EMPTY(ext)); + ret = g_strdup_printf("%s-%s%s", virDomainGetName(dom), + nowstr, NULLSTR_EMPTY(ext)); return ret; } diff --git a/tools/virsh-network.c b/tools/virsh-network.c index a02c85fcb1..ed90736345 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1433,11 +1433,10 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) const char *typestr = NULL; g_autofree char *cidr_format = NULL; virNetworkDHCPLeasePtr lease = leases[i]; - time_t expirytime_tmp = lease->expirytime; - struct tm ts; - char expirytime[32]; - localtime_r(&expirytime_tmp, &ts); - strftime(expirytime, sizeof(expirytime), "%Y-%m-%d %H:%M:%S", &ts); + g_autoptr(GDateTime) then = g_date_time_new_from_unix_local(lease->expirytime); + g_autofree char *thenstr = NULL; + + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S"); if (lease->type == VIR_IP_ADDR_TYPE_IPV4) typestr = "ipv4"; @@ -1447,7 +1446,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) cidr_format = g_strdup_printf("%s/%d", lease->ipaddr, lease->prefix); if (vshTableRowAppend(table, - expirytime, + thenstr, NULLSTR_MINUS(lease->mac), NULLSTR_MINUS(typestr), NULLSTR_MINUS(cidr_format), diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index a42397b42e..d5e68e4b18 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1492,9 +1492,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) virDomainSnapshotPtr snapshot = NULL; char *state = NULL; long long creation_longlong; - time_t creation_time_t; - char timestr[100]; - struct tm time_info; + g_autoptr(GDateTime) then = NULL; + g_autofree gchar *thenstr = NULL; bool tree = vshCommandOptBool(cmd, "tree"); bool name = vshCommandOptBool(cmd, "name"); bool from = vshCommandOptBool(cmd, "from"); @@ -1624,22 +1623,16 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (virXPathLongLong("string(/domainsnapshot/creationTime)", ctxt, &creation_longlong) < 0) continue; - creation_time_t = creation_longlong; - if (creation_time_t != creation_longlong) { - vshError(ctl, "%s", _("time_t overflow")); - continue; - } - localtime_r(&creation_time_t, &time_info); - strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z", - &time_info); + then = g_date_time_new_from_unix_local(creation_longlong); + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S %z"); if (parent) { - if (vshTableRowAppend(table, snap_name, timestr, state, + if (vshTableRowAppend(table, snap_name, thenstr, state, NULLSTR_EMPTY(parent_snap), NULL) < 0) goto cleanup; } else { - if (vshTableRowAppend(table, snap_name, timestr, state, + if (vshTableRowAppend(table, snap_name, thenstr, state, NULL) < 0) goto cleanup; } diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 30106d1971..c71ebb1012 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -69,40 +69,6 @@ vshAdmClientTransportToString(int transport) return str ? _(str) : _("unknown"); } -/* - * vshAdmGetTimeStr: - * - * Produces string representation (local time) of @then - * (seconds since epoch UTC) using format 'YYYY-MM-DD HH:MM:SS+ZZZZ'. - * - * Returns 0 if conversion finished successfully, -1 in case of an error. - * Caller is responsible for freeing the string returned. - */ -static int -vshAdmGetTimeStr(vshControl *ctl, time_t then, char **result) -{ - char *tmp = NULL; - struct tm timeinfo; - - if (!localtime_r(&then, &timeinfo)) - goto error; - - if (VIR_ALLOC_N(tmp, VIRT_ADMIN_TIME_BUFLEN) < 0) - goto error; - - if (strftime(tmp, VIRT_ADMIN_TIME_BUFLEN, "%Y-%m-%d %H:%M:%S%z", - &timeinfo) == 0) { - VIR_FREE(tmp); - goto error; - } - - *result = tmp; - return 0; - - error: - vshError(ctl, "%s", _("Timestamp string conversion failed")); - return -1; -} /* * vshAdmCatchDisconnect: @@ -646,19 +612,19 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) goto cleanup; for (i = 0; i < nclts; i++) { - g_autofree char *timestr = NULL; + g_autoptr(GDateTime) then = NULL; + g_autofree gchar *thenstr = NULL; g_autofree char *idStr = NULL; virAdmClientPtr client = clts[i]; id = virAdmClientGetID(client); + then = g_date_time_new_from_unix_local(virAdmClientGetTimestamp(client)); transport = virAdmClientGetTransport(client); - if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(client), - ×tr) < 0) - goto cleanup; + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S%z"); idStr = g_strdup_printf("%llu", id); if (vshTableRowAppend(table, idStr, vshAdmClientTransportToString(transport), - timestr, NULL) < 0) + thenstr, NULL) < 0) goto cleanup; } @@ -714,7 +680,8 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) size_t i; unsigned long long id; const char *srvname = NULL; - char *timestr = NULL; + g_autoptr(GDateTime) then = NULL; + g_autofree gchar *thenstr = NULL; virAdmServerPtr srv = NULL; virAdmClientPtr clnt = NULL; virTypedParameterPtr params = NULL; @@ -739,12 +706,13 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(clnt), ×tr) < 0) - goto cleanup; + + then = g_date_time_new_from_unix_local(virAdmClientGetTimestamp(clnt)); + thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S%z"); /* this info is provided by the client object itself */ vshPrint(ctl, "%-15s: %llu\n", "id", virAdmClientGetID(clnt)); - vshPrint(ctl, "%-15s: %s\n", "connection_time", timestr); + vshPrint(ctl, "%-15s: %s\n", "connection_time", thenstr); vshPrint(ctl, "%-15s: %s\n", "transport", vshAdmClientTransportToString(virAdmClientGetTransport(clnt))); @@ -760,7 +728,6 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd) virTypedParamsFree(params, nparams); virAdmServerFree(srv); virAdmClientFree(clnt); - VIR_FREE(timestr); return ret; } diff --git a/tools/vsh.c b/tools/vsh.c index 5c8908f240..949c8dad7b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2182,8 +2182,8 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, char *str = NULL; size_t len; const char *lvl = ""; - time_t stTime; - struct tm stTm; + g_autoptr(GDateTime) now = g_date_time_new_now_local(); + g_autofree gchar *nowstr = NULL; if (ctl->log_fd == -1) return; @@ -2193,15 +2193,9 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, * * [YYYY.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message */ - time(&stTime); - localtime_r(&stTime, &stTm); - virBufferAsprintf(&buf, "[%d.%02d.%02d %02d:%02d:%02d %s %d] ", - (1900 + stTm.tm_year), - (1 + stTm.tm_mon), - stTm.tm_mday, - stTm.tm_hour, - stTm.tm_min, - stTm.tm_sec, + nowstr = g_date_time_format(now, "%Y.%m.%d %H:%M:%S"); + virBufferAsprintf(&buf, "[%s %s %d] ", + nowstr, ctl->progname, (int) getpid()); switch (log_level) { -- 2.24.1