Prepare for linking with glib by probing for it at configure time. Per supported platforms target, the min glib versions on relevant distros are: RHEL-8: 2.56.1 RHEL-7: 2.50.3 Debian (Buster): 2.58.3 Debian (Stretch): 2.50.3 OpenBSD (Ports): 2.58.3 FreeBSD (Ports): 2.56.3 OpenSUSE Leap 15: 2.54.3 SLE12-SP2: 2.48.2 Ubuntu (Xenial): 2.48.0 macOS (Homebrew): 2.56.0 This suggests that a minimum glib of 2.48 is a reasonable target. This aligns with the minimum version required by qemu too. Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- configure.ac | 2 ++ libvirt.spec.in | 1 + m4/virt-glib.m4 | 36 ++++++++++++++++++++++++++++++++++++ mingw-libvirt.spec.in | 2 ++ 4 files changed, 41 insertions(+) create mode 100644 m4/virt-glib.m4 diff --git a/configure.ac b/configure.ac index 40e93e251e..81f061f058 100644 --- a/configure.ac +++ b/configure.ac @@ -311,6 +311,7 @@ LIBVIRT_CHECK_DLOPEN LIBVIRT_CHECK_FIREWALLD LIBVIRT_CHECK_FIREWALLD_ZONE LIBVIRT_CHECK_FUSE +LIBVIRT_CHECK_GLIB LIBVIRT_CHECK_GLUSTER LIBVIRT_CHECK_GNUTLS LIBVIRT_CHECK_HAL @@ -1001,6 +1002,7 @@ LIBVIRT_RESULT_DLOPEN LIBVIRT_RESULT_FIREWALLD LIBVIRT_RESULT_FIREWALLD_ZONE LIBVIRT_RESULT_FUSE +LIBVIRT_RESULT_GLIB LIBVIRT_RESULT_GLUSTER LIBVIRT_RESULT_GNUTLS LIBVIRT_RESULT_HAL diff --git a/libvirt.spec.in b/libvirt.spec.in index 7f5183f341..dcad08cb5f 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -273,6 +273,7 @@ BuildRequires: systemd-units %if %{with_libxl} BuildRequires: xen-devel %endif +BuildRequires: glib2-devel >= 2.48 BuildRequires: libxml2-devel BuildRequires: libxslt BuildRequires: readline-devel diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4 new file mode 100644 index 0000000000..5a5bc19660 --- /dev/null +++ b/m4/virt-glib.m4 @@ -0,0 +1,36 @@ +dnl The glib.so library +dnl +dnl Copyright (C) 2016 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>;. +dnl + +AC_DEFUN([LIBVIRT_ARG_GLIB], [ + LIBVIRT_ARG_WITH([GLIB], [glib-2.0 location], [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_GLIB],[ + GLIB_REQUIRED=2.48.0 + + LIBVIRT_CHECK_PKG([GLIB], [glib-2.0], [$GLIB_REQUIRED]) + + if test "$with_glib" = "no" ; then + AC_MSG_ERROR([glib-2.0 >= $GLIB_REQUIRED is required for libvirt]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_GLIB], [ + LIBVIRT_RESULT_LIB([GLIB]) +]) diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index a20c4b7d74..c29f3eeed2 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -52,6 +52,8 @@ BuildRequires: mingw32-gcc BuildRequires: mingw64-gcc BuildRequires: mingw32-binutils BuildRequires: mingw64-binutils +BuildRequires: mingw32-glib2 >= 2.48 +BuildRequires: mingw64-glib2 >= 2.48 BuildRequires: mingw32-libgpg-error BuildRequires: mingw64-libgpg-error BuildRequires: mingw32-libgcrypt -- 2.21.0

Add the main glib.h to internal.h so that all common code can use it. Historically glib allowed applications to register an alternative memory allocator, so mixing g_malloc/g_free with malloc/free was not safe. This was feature was dropped in 2.46.0 with: commit 3be6ed60aa58095691bd697344765e715a327fc1 Author: Alexander Larsson <alexl(a)redhat.com&gt; Date: Sat Jun 27 18:38:42 2015 +0200 Deprecate and drop support for memory vtables Applications are still encourged to match g_malloc/g_free, but it is no longer a mandatory requirement for correctness, just stylistic. This is explicitly clarified in commit 1f24b36607bf708f037396014b2cdbc08d67b275 Author: Daniel P. Berrangé <berrange(a)redhat.com&gt; Date: Thu Sep 5 14:37:54 2019 +0100 gmem: clarify that g_malloc always uses the system allocator Applications can still use custom allocators in general, but they must do this by linking to a library that replaces the core malloc/free implemenentation entirely, instead of via a glib specific call. This means that libvirt does not need to be concerned about use of g_malloc/g_free causing an ABI change in the public libary, and can avoid memory copying when talking to external libraries. This patch probes for glib, which provides the foundation layer with a collection of data structures, helper APIs, and platform portability logic. Later patches will introduce linkage to gobject which provides the object type system, built on glib, and gio which providing objects for various interesting tasks, most notably including DBus client and server support and portable sockets APIs, but much more too. Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/hacking.html.in | 21 +++++++++++++++++++++ src/Makefile.am | 2 ++ src/internal.h | 1 + src/lxc/Makefile.inc.am | 2 ++ src/remote/Makefile.inc.am | 1 + src/util/Makefile.inc.am | 1 + tests/Makefile.am | 3 ++- tools/Makefile.am | 1 + 8 files changed, 31 insertions(+), 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index edf2f54ce3..93b451591e 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -989,6 +989,27 @@ BAD: it points to, or it is aliased to another pointer that is. </p> + <h2><a id="glib">Adoption of GLib APIs</a></h2> + + <p> + Libvirt has adopted use of the + <a href="https://developer.gnome.org/glib/stable/">GLib library</a>. + Due to libvirt's long history of development, there are many APIs + in libvirt, for which GLib provides an alternative solution. The + general rule to follow is that the standard GLib solution will be + preferred over historical libvirt APIs. Existing code will be + ported over to use GLib APIs over time, but new code should use + the GLib APIs straight away where possible. + </p> + + <p> + The following is a list of libvirt APIs that should no longer be + used in new code, and their suggested GLib replacements: + </p> + + <dl> + </dl> + <h2><a id="memalloc">Low level memory management</a></h2> <p> diff --git a/src/Makefile.am b/src/Makefile.am index bd03b09cb2..9d21395892 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -34,6 +34,7 @@ AM_CPPFLAGS = -I../gnulib/lib \ WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS) AM_CFLAGS = $(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ @@ -558,6 +559,7 @@ libvirt_admin_la_LIBADD += \ $(YAJL_LIBS) \ $(DEVMAPPER_LIBS) \ $(LIBXML_LIBS) \ + $(GLIB_LIBS) \ $(SSH2_LIBS) \ $(SASL_LIBS) \ $(GNUTLS_LIBS) \ diff --git a/src/internal.h b/src/internal.h index e1a69be9f2..56e99241b0 100644 --- a/src/internal.h +++ b/src/internal.h @@ -28,6 +28,7 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> +#include <glib.h> #if STATIC_ANALYSIS # undef NDEBUG /* Don't let a prior NDEBUG definition cause trouble. */ diff --git a/src/lxc/Makefile.inc.am b/src/lxc/Makefile.inc.am index b4d560702c..0c9618e185 100644 --- a/src/lxc/Makefile.inc.am +++ b/src/lxc/Makefile.inc.am @@ -184,6 +184,7 @@ libvirt_lxc_LDFLAGS = \ $(PIE_LDFLAGS) \ $(CAPNG_LIBS) \ $(LIBXML_LIBS) \ + $(GLIB_LIBS) \ $(NULL) libvirt_lxc_LDADD = \ libvirt.la \ @@ -200,6 +201,7 @@ libvirt_lxc_CFLAGS = \ $(PIE_CFLAGS) \ $(CAPNG_CFLAGS) \ $(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(LIBNL_CFLAGS) \ $(FUSE_CFLAGS) \ $(DBUS_CFLAGS) \ diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index 5a5c90a922..e02c20d47c 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -38,6 +38,7 @@ REMOTE_DAEMON_SOURCES = \ REMOTE_DAEMON_CFLAGS = \ $(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ $(XDR_CFLAGS) \ diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index 9747816fac..454f8e1a8c 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -290,6 +290,7 @@ libvirt_util_la_LIBADD = \ $(DBUS_LIBS) \ $(WIN32_EXTRA_LIBS) \ $(LIBXML_LIBS) \ + $(GLIB_LIBS) \ $(SECDRIVER_LIBS) \ $(NUMACTL_LIBS) \ $(ACL_LIBS) \ diff --git a/tests/Makefile.am b/tests/Makefile.am index d88ad7f686..7b81ee88f1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,7 @@ AM_CFLAGS = \ -Dabs_srcdir="\"$(abs_srcdir)\"" \ -Dabs_top_srcdir="\"$(abs_top_srcdir)\"" \ $(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(LIBNL_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ @@ -525,7 +526,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS) libxlmock_la_SOURCES = \ libxlmock.c -libxlmock_la_CFLAGS = $(LIBXL_CFLAGS) $(LIBXML_CFLAGS) +libxlmock_la_CFLAGS = $(LIBXL_CFLAGS) $(LIBXML_CFLAGS) $(GLIB_CFLAGS) libxlmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) libxlmock_la_LIBADD = $(MOCKLIBS_LIBS) diff --git a/tools/Makefile.am b/tools/Makefile.am index ece70384e6..94fb4ecb8b 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -36,6 +36,7 @@ AM_CFLAGS = \ $(COVERAGE_CFLAGS) \ $(PIE_CFLAGS) \ $(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(NULL) AM_LDFLAGS = \ -- 2.21.0

Convert the VIR_ALLOC family of APIs with use of the g_malloc family of APIs. Use of VIR_ALLOC related functions should be incrementally phased out over time, allowing return value checks to be dropped. Use of VIR_FREE should be replaced with auto-cleanup whenever possible. We previously used the 'calloc-posix' gnulib module because mingw does not set errno to ENOMEM on failure. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- bootstrap.conf | 1 - docs/hacking.html.in | 108 ++++++------------------------------------- src/util/viralloc.c | 29 +++--------- src/util/viralloc.h | 9 ++++ 4 files changed, 28 insertions(+), 119 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index e40170a77b..b98fdedeb1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -26,7 +26,6 @@ byteswap c-ctype c-strcase c-strcasestr -calloc-posix canonicalize-lgpl chown clock-time diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 93b451591e..8072796312 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -990,7 +990,7 @@ BAD: </p> <h2><a id="glib">Adoption of GLib APIs</a></h2> - + <p> Libvirt has adopted use of the <a href="https://developer.gnome.org/glib/stable/">GLib library</a>. @@ -1008,102 +1008,20 @@ BAD: </p> <dl> + <dt>VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N, + VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT, + VIR_DELETE_ELEMENT</dt> + <dd>Prefer the GLib APIs g_new0/g_renew/g_free in most cases. + There should rarely be a need to use g_malloc/g_realloc. + Instead of using plain C arrays, it is preferrable to use + one of the GLib types, GArray, GPtrArray or GByteArray. These + all use a struct to track the array memory and size together + and efficiently resize. <strong>NEVER MIX</strong> use of the + classic libvirt memory allocation APIs and GLib APIs within + a single method. Keep the style consistent, converting existing + code to GLib style in a separate, prior commit.</dd> </dl> - <h2><a id="memalloc">Low level memory management</a></h2> - - <p> - Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt - codebase, because they encourage a number of serious coding bugs and do - not enable compile time verification of checks for NULL. Instead of these - routines, use the macros from viralloc.h. - </p> - - <ul> - <li><p>To allocate a single object:</p> - -<pre> - virDomainPtr domain; - - if (VIR_ALLOC(domain) &lt; 0) - return NULL; -</pre> - </li> - - <li><p>To allocate an array of objects:</p> -<pre> - virDomainPtr domains; - size_t ndomains = 10; - - if (VIR_ALLOC_N(domains, ndomains) &lt; 0) - return NULL; -</pre> - </li> - - <li><p>To allocate an array of object pointers:</p> -<pre> - virDomainPtr *domains; - size_t ndomains = 10; - - if (VIR_ALLOC_N(domains, ndomains) &lt; 0) - return NULL; -</pre> - </li> - - <li><p>To re-allocate the array of domains to be 1 element - longer (however, note that repeatedly expanding an array by 1 - scales quadratically, so this is recommended only for smaller - arrays):</p> -<pre> - virDomainPtr domains; - size_t ndomains = 0; - - if (VIR_EXPAND_N(domains, ndomains, 1) &lt; 0) - return NULL; - domains[ndomains - 1] = domain; -</pre></li> - - <li><p>To ensure an array has room to hold at least one more - element (this approach scales better, but requires tracking - allocation separately from usage)</p> - -<pre> - virDomainPtr domains; - size_t ndomains = 0; - size_t ndomains_max = 0; - - if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) &lt; 0) - return NULL; - domains[ndomains++] = domain; -</pre> - </li> - - <li><p>To trim an array of domains from its allocated size down - to the actual used size:</p> - -<pre> - virDomainPtr domains; - size_t ndomains = x; - size_t ndomains_max = y; - - VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains); -</pre></li> - - <li><p>To free an array of domains:</p> -<pre> - virDomainPtr domains; - size_t ndomains = x; - size_t ndomains_max = y; - size_t i; - - for (i = 0; i &lt; ndomains; i++) - VIR_FREE(domains[i]); - VIR_FREE(domains); - ndomains_max = ndomains = 0; -</pre> - </li> - </ul> - <h2><a id="file_handling">File handling</a></h2> <p> diff --git a/src/util/viralloc.c b/src/util/viralloc.c index 10a8d0fb73..b8ca850764 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c @@ -45,10 +45,7 @@ VIR_LOG_INIT("util.alloc"); int virAlloc(void *ptrptr, size_t size) { - *(void **)ptrptr = calloc(1, size); - if (*(void **)ptrptr == NULL) - abort(); - + *(void **)ptrptr = g_malloc0(size); return 0; } @@ -69,10 +66,7 @@ int virAllocN(void *ptrptr, size_t size, size_t count) { - *(void**)ptrptr = calloc(count, size); - if (*(void**)ptrptr == NULL) - abort(); - + *(void**)ptrptr = g_malloc0_n(count, size); return 0; } @@ -94,16 +88,7 @@ int virReallocN(void *ptrptr, size_t size, size_t count) { - void *tmp; - - if (xalloc_oversized(count, size)) - abort(); - - tmp = realloc(*(void**)ptrptr, size * count); - if (!tmp && ((size * count) != 0)) - abort(); - - *(void**)ptrptr = tmp; + *(void **)ptrptr = g_realloc_n(*(void**)ptrptr, size, count); return 0; } @@ -343,9 +328,7 @@ int virAllocVar(void *ptrptr, abort(); alloc_size = struct_size + (element_size * count); - *(void **)ptrptr = calloc(1, alloc_size); - if (*(void **)ptrptr == NULL) - abort(); + *(void **)ptrptr = g_malloc0(alloc_size); return 0; } @@ -362,7 +345,7 @@ void virFree(void *ptrptr) { int save_errno = errno; - free(*(void**)ptrptr); + g_free(*(void**)ptrptr); *(void**)ptrptr = NULL; errno = save_errno; } @@ -395,7 +378,7 @@ void virDispose(void *ptrptr, if (*(void**)ptrptr && count > 0) memset(*(void **)ptrptr, 0, count * element_size); - free(*(void**)ptrptr); + g_free(*(void**)ptrptr); *(void**)ptrptr = NULL; if (countptr) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 3e72e40bc9..517f9aada6 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -24,6 +24,15 @@ #include "internal.h" +/** + * DEPRECATION WARNING + * + * APIs in this file should only be used when modifying existing code. + * Consider converting existing code to use the new APIs when touching + * it. All new code must use the GLib memory allocation APIs and/or + * GLib array data types. See the hacking file for more guidance. + */ + /* Return 1 if an array of N objects, each of size S, cannot exist due to size arithmetic overflow. S must be positive and N must be nonnegative. This is a macro, not an inline function, so that it -- 2.21.0

Convert the string duplication APIs to use the g_strdup family of APIs. We previously used the 'strdup-posix' gnulib module because mingw does not set errno to ENOMEM on failure We previously used the 'strndup' gnulib module because this function does not exist on mingw. We previously used the 'vasprintf' gnulib module because of many GNU supported format specifiers not working on non-Linux platforms. glib's own equivalent standardizes on GNU format specifiers too. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- bootstrap.conf | 3 --- docs/hacking.html.in | 8 ++++++++ src/util/virstring.c | 28 ++++++++++++++++++++++------ src/util/virstring.h | 8 ++++++++ 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index b98fdedeb1..7105ae2eeb 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -82,8 +82,6 @@ snprintf socket stat-time strchrnul -strdup-posix -strndup strerror strerror_r-posix strptime @@ -99,7 +97,6 @@ uname unsetenv useless-if-before-free usleep -vasprintf verify vc-list-files vsnprintf diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 8072796312..5839464e99 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1020,6 +1020,14 @@ BAD: classic libvirt memory allocation APIs and GLib APIs within a single method. Keep the style consistent, converting existing code to GLib style in a separate, prior commit.</dd> + + <dt>VIR_STRDUP, VIR_STRNDUP</dt> + <dd>Prefer the GLib APIs g_strdup and g_strndup.</dd> + + <dt>virAsprintf, virVasprintf</dt> + <dd>The GLib APIs g_strdup_printf / g_strdup_vprint can be used, + but beware that they don't abort on OOM, so the libvirt wrappers + may still be desirable to use. Don't use g_vasprintf or g_asprintf.</dd> </dl> <h2><a id="file_handling">File handling</a></h2> diff --git a/src/util/virstring.c b/src/util/virstring.c index a4cc7e9c0a..6b2b6ed24e 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -18,6 +18,7 @@ #include <config.h> +#include <glib/gprintf.h> #include <regex.h> #include <locale.h> @@ -730,10 +731,21 @@ virVasprintfInternal(char **strp, const char *fmt, va_list list) { + char *str = NULL; int ret; - if ((ret = vasprintf(strp, fmt, list)) == -1) + ret = g_vasprintf(&str, fmt, list); + + /* GLib is supposed to abort() on OOM, but a mistake meant + * it did not. Delete this once our min glib is at 2.64.0 + * which includes the fix: + * https://gitlab.gnome.org/GNOME/glib/merge_requests/1145 + */ +#if !GLIB_CHECK_VERSION(2, 64, 0) + if (!str) abort(); +#endif + *strp = str; return ret; } @@ -743,11 +755,17 @@ virAsprintfInternal(char **strp, const char *fmt, ...) { va_list ap; + char *str = NULL; int ret; va_start(ap, fmt); - ret = virVasprintfInternal(strp, fmt, ap); + ret = g_vasprintf(&str, fmt, ap); va_end(ap); + + if (!*str) + abort(); + *strp = str; + return ret; } @@ -936,8 +954,7 @@ virStrdup(char **dest, *dest = NULL; if (!src) return 0; - if (!(*dest = strdup(src))) - abort(); + *dest = g_strdup(src); return 1; } @@ -965,8 +982,7 @@ virStrndup(char **dest, return 0; if (n < 0) n = strlen(src); - if (!(*dest = strndup(src, n))) - abort(); + *dest = g_strndup(src, n); return 1; } diff --git a/src/util/virstring.h b/src/util/virstring.h index f537f3472e..3ffe51f7b8 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -145,6 +145,8 @@ int virVasprintfInternal(char **strp, const char *fmt, va_list list) * @dst: variable to hold result (char*, not char**) * @src: string to duplicate * + * DEPRECATED: use g_strdup instead + * * Duplicate @src string and store it into @dst. * * This macro is safe to use on arguments with side effects. @@ -158,6 +160,8 @@ int virVasprintfInternal(char **strp, const char *fmt, va_list list) * @dst: variable to hold result (char*, not char**) * @src: string to duplicate * + * DEPRECATED: use g_strdup instead + * * Duplicate @src string and store it into @dst. * * This macro is safe to use on arguments with side effects. @@ -172,6 +176,8 @@ int virVasprintfInternal(char **strp, const char *fmt, va_list list) * @src: string to duplicate * @n: the maximum number of bytes to copy * + * DEPRECATED: use g_strndup instead + * * Duplicate @src string and store it into @dst. If @src is longer than @n, * only @n bytes are copied and terminating null byte '\0' is added. If @n * is a negative number, then the whole @src string is copied. That is, @@ -189,6 +195,8 @@ int virVasprintfInternal(char **strp, const char *fmt, va_list list) * @src: string to duplicate * @n: the maximum number of bytes to copy * + * DEPRECATED: use g_strndup instead + * * Duplicate @src string and store it into @dst. If @src is longer than @n, * only @n bytes are copied and terminating null byte '\0' is added. If @n * is a negative number, then the whole @src string is copied. That is, -- 2.21.0

On Tue, Oct 08, 2019 at 03:49:50PM +0200, Ján Tomko wrote: yeah, my bad. I blindly assumed there was a non-valist variant. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Mon, Oct 07, 2019 at 06:14:08PM +0100, Daniel P. Berrangé wrote: vice-versa HACKING vice-versa Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

On Mon, Oct 07, 2019 at 06:14:12PM +0100, Daniel P. Berrangé wrote: Unrelated change. Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

On Mon, Oct 07, 2019 at 06:14:17PM +0100, Daniel P. Berrangé wrote: Extra whitespace Consider not mixing camel case with snake case. Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

On Wed, Oct 09, 2019 at 09:07:57AM +0200, Pavel Hrdina wrote: Well we have a choice of not doing that :) Actually, G_DEFINE_TYPE is documented as taking one camelCase argument and one snake_case argument for the type name: https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#... Jano

Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libxl/libxl_capabilities.c | 42 +++++++++++++++------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 73ae0b3fa1..65c68ffb52 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -21,7 +21,6 @@ #include <config.h> #include <libxl.h> -#include <regex.h> #include "internal.h" #include "virlog.h" @@ -374,10 +373,10 @@ static int libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) { const libxl_version_info *ver_info; - int err; - regex_t regex; + g_autoptr(GRegex) regex = NULL; + g_autoptr(GError) err = NULL; + g_autoptr(GMatchInfo) info = NULL; char *str, *token; - regmatch_t subs[4]; char *saveptr = NULL; size_t i; @@ -398,12 +397,10 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) return -1; } - err = regcomp(&regex, XEN_CAP_REGEX, REG_EXTENDED); - if (err != 0) { - char error[100]; - regerror(err, &regex, error, sizeof(error)); + regex = g_regex_new(XEN_CAP_REGEX, G_REGEX_EXTENDED, 0, &err); + if (!regex) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to compile regex %s"), error); + _("Failed to compile regex %s"), err->message); return -1; } @@ -436,31 +433,31 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0]) && (token = strtok_r(str, " ", &saveptr)) != NULL; str = NULL) { - if (regexec(&regex, token, sizeof(subs) / sizeof(subs[0]), - subs, 0) == 0) { - int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm"); + if (g_regex_match(regex, token, 0, &info)) { + g_autofree char *modestr = g_match_info_fetch(info, 1); + g_autofree char *archstr = g_match_info_fetch(info, 2); + g_autofree char *suffixstr = g_match_info_fetch(info, 3); + int hvm = STRPREFIX(modestr, "hvm"); virArch arch; int pae = 0, nonpae = 0, ia64_be = 0; - if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) { + if (STRPREFIX(archstr, "x86_32")) { arch = VIR_ARCH_I686; - if (subs[3].rm_so != -1 && - STRPREFIX(&token[subs[3].rm_so], "p")) + if (suffixstr != NULL && STRPREFIX(suffixstr, "p")) pae = 1; else nonpae = 1; - } else if (STRPREFIX(&token[subs[2].rm_so], "x86_64")) { + } else if (STRPREFIX(archstr, "x86_64")) { arch = VIR_ARCH_X86_64; - } else if (STRPREFIX(&token[subs[2].rm_so], "ia64")) { + } else if (STRPREFIX(archstr, "ia64")) { arch = VIR_ARCH_ITANIUM; - if (subs[3].rm_so != -1 && - STRPREFIX(&token[subs[3].rm_so], "be")) + if (suffixstr != NULL && STRPREFIX(suffixstr, "be")) ia64_be = 1; - } else if (STRPREFIX(&token[subs[2].rm_so], "powerpc64")) { + } else if (STRPREFIX(archstr, "powerpc64")) { arch = VIR_ARCH_PPC64; - } else if (STRPREFIX(&token[subs[2].rm_so], "armv7l")) { + } else if (STRPREFIX(archstr, "armv7l")) { arch = VIR_ARCH_ARMV7L; - } else if (STRPREFIX(&token[subs[2].rm_so], "aarch64")) { + } else if (STRPREFIX(archstr, "aarch64")) { arch = VIR_ARCH_AARCH64; } else { continue; @@ -515,7 +512,6 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps) #endif } } - regfree(&regex); for (i = 0; i < nr_guest_archs; ++i) { virCapsGuestPtr guest; -- 2.21.0

Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/domain_event.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b33589f472..f7d1a38e46 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -22,8 +22,6 @@ #include <config.h> -#include <regex.h> - #include "domain_event.h" #include "object_event.h" #include "object_event_private.h" @@ -2009,7 +2007,7 @@ virDomainQemuMonitorEventNew(int id, * deregisters. */ struct virDomainQemuMonitorEventData { char *event; - regex_t regex; + GRegex *regex; unsigned int flags; void *opaque; virFreeCallback freecb; @@ -2241,7 +2239,7 @@ virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED, if (data->flags == -1) return true; if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) - return regexec(&data->regex, monitorEvent->event, 0, NULL, 0) == 0; + return g_regex_match(data->regex, monitorEvent->event, 0, NULL) == TRUE; if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) return STRCASEEQ(monitorEvent->event, data->event); return STREQ(monitorEvent->event, data->event); @@ -2255,7 +2253,7 @@ virDomainQemuMonitorEventCleanup(void *opaque) VIR_FREE(data->event); if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) - regfree(&data->regex); + g_regex_unref(data->regex); if (data->freecb) (data->freecb)(data->opaque); VIR_FREE(data); @@ -2306,20 +2304,17 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, return -1; data->flags = flags; if (event && flags != -1) { - int rflags = REG_NOSUB; - - if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) - rflags |= REG_ICASE; if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) { - int err = regcomp(&data->regex, event, rflags); + int cflags = 0; + g_autoptr(GError) err = NULL; - if (err) { - char error[100]; - regerror(err, &data->regex, error, sizeof(error)); + if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) + cflags |= G_REGEX_CASELESS; + data->regex = g_regex_new(event, cflags, 0, &err); + if (!data->regex) { virReportError(VIR_ERR_INVALID_ARG, _("failed to compile regex '%s': %s"), - event, error); - regfree(&data->regex); + event, err->message); VIR_FREE(data); return -1; } -- 2.21.0

Daniel P. Berrangé <berrange(a)redhat.com&gt; [2019-10-07, 06:14PM +0100]: Do we want to use G_REGEX_OPTIMIZE here? -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

The GOptionContext API has the benefit over getopt_long that it will automatically handle --help output formatting. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virsh.c | 304 +++++++++++++++++++++++--------------------------- 1 file changed, 137 insertions(+), 167 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ec20f35a77..49aca96a8d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -23,7 +23,6 @@ #include <stdarg.h> #include <unistd.h> -#include <getopt.h> #include <sys/time.h> #include <fcntl.h> #include <time.h> @@ -445,53 +444,36 @@ virshDeinit(vshControl *ctl) } /* - * Print usage + * Build help description for commands */ -static void -virshUsage(void) +static char * +virshBuildDescription(void) { const vshCmdGrp *grp; const vshCmdDef *cmd; - - fprintf(stdout, _("\n%s [options]... [<command_string>]" - "\n%s [options]... <command> [args...]\n\n" - " options:\n" - " -c | --connect=URI hypervisor connection URI\n" - " -d | --debug=NUM debug level [0-4]\n" - " -e | --escape <char> set escape sequence for console\n" - " -h | --help this help\n" - " -k | --keepalive-interval=NUM\n" - " keepalive interval in seconds, 0 for disable\n" - " -K | --keepalive-count=NUM\n" - " number of possible missed keepalive messages\n" - " -l | --log=FILE output logging to file\n" - " -q | --quiet quiet mode\n" - " -r | --readonly connect readonly\n" - " -t | --timing print timing information\n" - " -v short version\n" - " -V long version\n" - " --version[=TYPE] version, TYPE is short or long (default short)\n" - " commands (non interactive mode):\n\n"), progname, - progname); + GString *str = g_string_new("Commands (non interactive mode):\n\n"); for (grp = cmdGroups; grp->name; grp++) { - fprintf(stdout, _(" %s (help keyword '%s')\n"), - grp->name, grp->keyword); + g_string_append_printf(str, + _(" %s (help keyword '%s')\n"), + grp->name, grp->keyword); for (cmd = grp->commands; cmd->name; cmd++) { if (cmd->flags & VSH_CMD_FLAG_ALIAS) continue; - fprintf(stdout, - " %-30s %s\n", cmd->name, - _(vshCmddefGetInfo(cmd, "help"))); + g_string_append_printf(str, + " %-30s %s\n", + cmd->name, + _(vshCmddefGetInfo(cmd, "help"))); } - fprintf(stdout, "\n"); + g_string_append_printf(str, "\n"); } - fprintf(stdout, "%s", - _("\n (specify help <group> for details about the commands in the group)\n")); - fprintf(stdout, "%s", - _("\n (specify help <command> for details about the command)\n\n")); - return; + g_string_append_printf(str, + _("Specify help <group> for details about the commands in the group)\n")); + g_string_append_printf(str, + _("Specify help <command> for details about the command)\n")); + + return g_string_free(str, FALSE); } /* @@ -647,6 +629,22 @@ virshAllowedEscapeChar(char c) ('@' <= c && c <= '_'); } +static gboolean +virshVersion(const gchar *option_name G_GNUC_UNUSED, + const gchar *value, + gpointer data, + GError **error G_GNUC_UNUSED) +{ + vshControl *ctl = data; + + if (STREQ(option_name, "-V") || STREQ_NULLABLE(value, "long")) + virshShowVersion(ctl); + else + puts(VERSION); + + exit(EXIT_SUCCESS); +} + /* * argv[]: virsh [options] [command] * @@ -654,152 +652,124 @@ virshAllowedEscapeChar(char c) static bool virshParseArgv(vshControl *ctl, int argc, char **argv) { - int arg, len, debug, keepalive; - size_t i; - int longindex = -1; + int debug = 0; + size_t len; virshControlPtr priv = ctl->privData; - struct option opt[] = { - {"connect", required_argument, NULL, 'c'}, - {"debug", required_argument, NULL, 'd'}, - {"escape", required_argument, NULL, 'e'}, - {"help", no_argument, NULL, 'h'}, - {"keepalive-interval", required_argument, NULL, 'k'}, - {"keepalive-count", required_argument, NULL, 'K'}, - {"log", required_argument, NULL, 'l'}, - {"quiet", no_argument, NULL, 'q'}, - {"readonly", no_argument, NULL, 'r'}, - {"timing", no_argument, NULL, 't'}, - {"version", optional_argument, NULL, 'v'}, - {NULL, 0, NULL, 0} + char *logfile = NULL; + int keepalive_interval = INT_MAX; + int keepalive_count = INT_MAX; + GOptionEntry opt[] = { + { "connect", 'c', 0, + G_OPTION_ARG_STRING, &ctl->connname, + _("hypervisor connection URI"), "URI" }, + { "debug", 'd', 0, + G_OPTION_ARG_INT, &debug, + _("debug level [0-4]\n"), "LEVEL" }, + { "escape", 'e', 0, + G_OPTION_ARG_STRING, &priv->escapeChar, + _("set escape sequence for console"), "ESCAPE" }, + { "keepalive-interval", 'k', 0, + G_OPTION_ARG_INT, &keepalive_interval, + _("keepalive interval in seconds, 0 for disable"), "SECS" }, + { "keepalive-count", 'K', 0, + G_OPTION_ARG_INT, &keepalive_count, + _("number of possible missed keepalive messages"), "NUM" }, + { "log", 'l', 0, + G_OPTION_ARG_STRING, &logfile, + _("output logging to file"), "FILENAME" }, + { "quiet", 'q', 0, + G_OPTION_ARG_NONE, &ctl->quiet, + _("quite mode"), NULL }, + { "readonly", 'r', 0, + G_OPTION_ARG_NONE, &priv->readonly, + _("connect readonly"), NULL }, + { "timing", 't', 0, + G_OPTION_ARG_NONE, &ctl->timing, + _("print timing information"), NULL }, + { "version", 'v', G_OPTION_FLAG_OPTIONAL_ARG, + G_OPTION_ARG_CALLBACK, virshVersion, + _("print short version"), "[short]" }, + { "version", 'V', G_OPTION_FLAG_OPTIONAL_ARG, + G_OPTION_ARG_CALLBACK, virshVersion, + _("print long version"), "long" }, + { NULL, 0, 0, 0, NULL, NULL, NULL }, }; + g_autoptr(GOptionContext) optctx = NULL; + GOptionGroup *optgrp; + g_autoptr(GError) error = NULL; + + optctx = g_option_context_new(_("[COMMAND [OPTION…] - libvirt shell")); + optgrp = g_option_group_new(NULL, NULL, NULL, ctl, NULL); + g_option_group_set_translation_domain(optgrp, PACKAGE); + g_option_group_add_entries(optgrp, opt); + g_option_context_set_main_group(optctx, optgrp); + g_option_context_set_strict_posix(optctx, true); + g_option_context_set_description(optctx, + virshBuildDescription()); + + if (!g_option_context_parse(optctx, &argc, &argv, &error)) { + vshError(ctl, _("option parsing failed: %s\n"), error->message); + exit(EXIT_FAILURE); + } - /* Standard (non-command) options. The leading + ensures that no - * argument reordering takes place, so that command options are - * not confused with top-level virsh options. */ - while ((arg = getopt_long(argc, argv, "+:c:d:e:hk:K:l:qrtvV", opt, &longindex)) != -1) { - switch (arg) { - case 'c': - VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, optarg); - break; - case 'd': - if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { - vshError(ctl, _("option %s takes a numeric argument"), - longindex == -1 ? "-d" : "--debug"); - exit(EXIT_FAILURE); - } - if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) - vshError(ctl, _("ignoring debug level %d out of range [%d-%d]"), - debug, VSH_ERR_DEBUG, VSH_ERR_ERROR); - else - ctl->debug = debug; - break; - case 'e': - len = strlen(optarg); - - if ((len == 2 && *optarg == '^' && - virshAllowedEscapeChar(optarg[1])) || - (len == 1 && *optarg != '^')) { - priv->escapeChar = optarg; - } else { - vshError(ctl, _("Invalid string '%s' for escape sequence"), - optarg); - exit(EXIT_FAILURE); - } - break; - case 'h': - virshUsage(); - exit(EXIT_SUCCESS); - break; - case 'k': - if (virStrToLong_i(optarg, NULL, 0, &keepalive) < 0) { - vshError(ctl, - _("Invalid value for option %s"), - longindex == -1 ? "-k" : "--keepalive-interval"); - exit(EXIT_FAILURE); - } + /* GOptionContext doesn't support -- explicitly, but + * we told it to stop at first unknown option, or + * first non-option, so we'll see '--' and can discard it + */ + if (argc >= 2 && STREQ(argv[1], "--")) { + argc--; + argv++; + } - if (keepalive < 0) { - vshError(ctl, - _("option %s requires a positive integer argument"), - longindex == -1 ? "-k" : "--keepalive-interval"); - exit(EXIT_FAILURE); - } - ctl->keepalive_interval = keepalive; - break; - case 'K': - if (virStrToLong_i(optarg, NULL, 0, &keepalive) < 0) { - vshError(ctl, - _("Invalid value for option %s"), - longindex == -1 ? "-K" : "--keepalive-count"); - exit(EXIT_FAILURE); - } + if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) { + vshError(ctl, _("debug level %d out of range [%d-%d]"), + debug, VSH_ERR_DEBUG, VSH_ERR_ERROR); + exit(EXIT_FAILURE); + } - if (keepalive < 0) { - vshError(ctl, - _("option %s requires a positive integer argument"), - longindex == -1 ? "-K" : "--keepalive-count"); - exit(EXIT_FAILURE); - } - ctl->keepalive_count = keepalive; - break; - case 'l': - vshCloseLogFile(ctl); - ctl->logfile = vshStrdup(ctl, optarg); - vshOpenLogFile(ctl); - break; - case 'q': - ctl->quiet = true; - break; - case 't': - ctl->timing = true; - break; - case 'r': - priv->readonly = true; - break; - case 'v': - if (STRNEQ_NULLABLE(optarg, "long")) { - puts(VERSION); - exit(EXIT_SUCCESS); - } - ATTRIBUTE_FALLTHROUGH; - case 'V': - virshShowVersion(ctl); - exit(EXIT_SUCCESS); - case ':': - for (i = 0; opt[i].name != NULL; i++) { - if (opt[i].val == optopt) - break; - } - if (opt[i].name) - vshError(ctl, _("option '-%c'/'--%s' requires an argument"), - optopt, opt[i].name); - else - vshError(ctl, _("option '-%c' requires an argument"), optopt); - exit(EXIT_FAILURE); - case '?': - if (optopt) - vshError(ctl, _("unsupported option '-%c'. See --help."), optopt); - else - vshError(ctl, _("unsupported option '%s'. See --help."), argv[optind - 1]); + if (keepalive_interval != INT_MAX) { + if (keepalive_interval < 0) { + vshError(ctl, + _("keepalive interval requires a positive integer")); exit(EXIT_FAILURE); - default: - vshError(ctl, _("unknown option")); + } + ctl->keepalive_interval = keepalive_interval; + } + + if (keepalive_count != INT_MAX) { + if (keepalive_count < 0) { + vshError(ctl, + _("keepalive count requires a positive integer")); exit(EXIT_FAILURE); } - longindex = -1; + ctl->keepalive_count = keepalive_count; + } + + len = strlen(priv->escapeChar); + if (!((len == 2 && *priv->escapeChar == '^' && + virshAllowedEscapeChar(priv->escapeChar[1])) || + (len == 1 && *priv->escapeChar != '^'))) { + vshError(ctl, _("Invalid string '%s' for escape sequence"), + priv->escapeChar); + exit(EXIT_FAILURE); + } + + if (logfile) { + vshCloseLogFile(ctl); + ctl->logfile = logfile; + vshOpenLogFile(ctl); } - if (argc == optind) { + if (argc == 1) { ctl->imode = true; } else { /* parse command */ ctl->imode = false; - if (argc - optind == 1) { - vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind], NULL); + if (argc == 2) { + vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[1]); + return vshCommandStringParse(ctl, argv[1], NULL); } else { - return vshCommandArgvParse(ctl, argc - optind, argv + optind); + return vshCommandArgvParse(ctl, argc - 1, argv + 1); } } return true; -- 2.21.0

The GOptionContext API has the benefit over getopt_long that it will automatically handle --help output formatting. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virt-admin.c | 210 ++++++++++++++++++++++----------------------- 1 file changed, 101 insertions(+), 109 deletions(-) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index e549ec1f83..bbb0cbb112 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -21,8 +21,6 @@ #include <config.h> #include "virt-admin.h" -#include <getopt.h> - #if WITH_READLINE # include <readline/readline.h> # include <readline/history.h> @@ -1212,44 +1210,34 @@ vshAdmDeinit(vshControl *ctl) /* * Print usage */ -static void -vshAdmUsage(void) +static char * +vshAdmBuildDescription(void) { const vshCmdGrp *grp; const vshCmdDef *cmd; - - fprintf(stdout, _("\n%s [options]... [<command_string>]" - "\n%s [options]... <command> [args...]\n\n" - " options:\n" - " -c | --connect=URI daemon admin connection URI\n" - " -d | --debug=NUM debug level [0-4]\n" - " -h | --help this help\n" - " -l | --log=FILE output logging to file\n" - " -q | --quiet quiet mode\n" - " -v short version\n" - " -V long version\n" - " --version[=TYPE] version, TYPE is short or long (default short)\n" - " commands (non interactive mode):\n\n"), progname, - progname); + GString *str = g_string_new("Commands (non interactive mode):\n\n"); for (grp = cmdGroups; grp->name; grp++) { - fprintf(stdout, _(" %s (help keyword '%s')\n"), - grp->name, grp->keyword); + g_string_append_printf(str, + _(" %s (help keyword '%s')\n"), + grp->name, grp->keyword); for (cmd = grp->commands; cmd->name; cmd++) { if (cmd->flags & VSH_CMD_FLAG_ALIAS) continue; - fprintf(stdout, - " %-30s %s\n", cmd->name, - _(vshCmddefGetInfo(cmd, "help"))); + g_string_append_printf(str, + " %-30s %s\n", + cmd->name, + _(vshCmddefGetInfo(cmd, "help"))); } - fprintf(stdout, "\n"); + g_string_append_printf(str, "\n"); } - fprintf(stdout, "%s", - _("\n (specify help <group> for details about the commands in the group)\n")); - fprintf(stdout, "%s", - _("\n (specify help <command> for details about the command)\n\n")); - return; + g_string_append_printf(str, + _("Specify help <group> for details about the commands in the group)\n")); + g_string_append_printf(str, + _("Specify help <command> for details about the command)\n")); + + return g_string_free(str, FALSE); } /* @@ -1275,103 +1263,107 @@ vshAdmShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) vshPrint(ctl, "\n"); } +static gboolean +vshAdmVersion(const gchar *option_name G_GNUC_UNUSED, + const gchar *value, + gpointer data, + GError **error G_GNUC_UNUSED) +{ + vshControl *ctl = data; + + if (STREQ(option_name, "-V") || STREQ_NULLABLE(value, "long")) + vshAdmShowVersion(ctl); + else + puts(VERSION); + + exit(EXIT_SUCCESS); +} + +/* + * argv[]: virsh [options] [command] + * + */ static bool vshAdmParseArgv(vshControl *ctl, int argc, char **argv) { - int arg, debug; - size_t i; - int longindex = -1; - struct option opt[] = { - {"connect", required_argument, NULL, 'c'}, - {"debug", required_argument, NULL, 'd'}, - {"help", no_argument, NULL, 'h'}, - {"log", required_argument, NULL, 'l'}, - {"quiet", no_argument, NULL, 'q'}, - {"version", optional_argument, NULL, 'v'}, - {NULL, 0, NULL, 0} + int debug = 0; + char *logfile = NULL; + GOptionEntry opt[] = { + { "connect", 'c', 0, + G_OPTION_ARG_STRING, &ctl->connname, + _("hypervisor connection URI"), "URI" }, + { "debug", 'd', 0, + G_OPTION_ARG_INT, &debug, + _("debug level [0-4]\n"), "LEVEL" }, + { "log", 'l', 0, + G_OPTION_ARG_STRING, &logfile, + _("output logging to file"), "FILENAME" }, + { "quiet", 'q', 0, + G_OPTION_ARG_NONE, &ctl->quiet, + _("quite mode"), NULL }, + { "version", 'v', G_OPTION_FLAG_OPTIONAL_ARG, + G_OPTION_ARG_CALLBACK, vshAdmVersion, + _("print short version"), "[short]" }, + { "version", 'V', G_OPTION_FLAG_OPTIONAL_ARG, + G_OPTION_ARG_CALLBACK, vshAdmVersion, + _("print long version"), "long" }, + { NULL, 0, 0, 0, NULL, NULL, NULL }, }; + g_autoptr(GOptionContext) optctx = NULL; + GOptionGroup *optgrp; + g_autoptr(GError) error = NULL; + + optctx = g_option_context_new(_("[COMMAND [OPTION…] - libvirt admin shell")); + optgrp = g_option_group_new(NULL, NULL, NULL, ctl, NULL); + g_option_group_set_translation_domain(optgrp, PACKAGE); + g_option_group_add_entries(optgrp, opt); + g_option_context_set_main_group(optctx, optgrp); + g_option_context_set_strict_posix(optctx, true); + g_option_context_set_description(optctx, + vshAdmBuildDescription()); + + if (!g_option_context_parse(optctx, &argc, &argv, &error)) { + vshError(ctl, _("option parsing failed: %s\n"), error->message); + exit(EXIT_FAILURE); + } - /* Standard (non-command) options. The leading + ensures that no - * argument reordering takes place, so that command options are - * not confused with top-level virt-admin options. */ - while ((arg = getopt_long(argc, argv, "+:c:d:hl:qvV", opt, &longindex)) != -1) { - switch (arg) { - case 'c': - VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, optarg); - break; - case 'd': - if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { - vshError(ctl, _("option %s takes a numeric argument"), - longindex == -1 ? "-d" : "--debug"); - exit(EXIT_FAILURE); - } - if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) - vshError(ctl, _("ignoring debug level %d out of range [%d-%d]"), - debug, VSH_ERR_DEBUG, VSH_ERR_ERROR); - else - ctl->debug = debug; - break; - case 'h': - vshAdmUsage(); - exit(EXIT_SUCCESS); - break; - case 'l': - vshCloseLogFile(ctl); - ctl->logfile = vshStrdup(ctl, optarg); - vshOpenLogFile(ctl); - break; - case 'q': - ctl->quiet = true; - break; - case 'v': - if (STRNEQ_NULLABLE(optarg, "long")) { - puts(VERSION); - exit(EXIT_SUCCESS); - } - ATTRIBUTE_FALLTHROUGH; - case 'V': - vshAdmShowVersion(ctl); - exit(EXIT_SUCCESS); - case ':': - for (i = 0; opt[i].name != NULL; i++) { - if (opt[i].val == optopt) - break; - } - if (opt[i].name) - vshError(ctl, _("option '-%c'/'--%s' requires an argument"), - optopt, opt[i].name); - else - vshError(ctl, _("option '-%c' requires an argument"), optopt); - exit(EXIT_FAILURE); - case '?': - if (optopt) - vshError(ctl, _("unsupported option '-%c'. See --help."), optopt); - else - vshError(ctl, _("unsupported option '%s'. See --help."), argv[optind - 1]); - exit(EXIT_FAILURE); - default: - vshError(ctl, _("unknown option")); - exit(EXIT_FAILURE); - } - longindex = -1; + /* GOptionContext doesn't support -- explicitly, but + * we told it to stop at first unknown option, or + * first non-option, so we'll see '--' and can discard it + */ + if (argc >= 2 && STREQ(argv[1], "--")) { + argc--; + argv++; } - if (argc == optind) { + if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) { + vshError(ctl, _("debug level %d out of range [%d-%d]"), + debug, VSH_ERR_DEBUG, VSH_ERR_ERROR); + exit(EXIT_FAILURE); + } + + if (logfile) { + vshCloseLogFile(ctl); + ctl->logfile = logfile; + vshOpenLogFile(ctl); + } + + if (argc == 1) { ctl->imode = true; } else { /* parse command */ ctl->imode = false; - if (argc - optind == 1) { - vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind], NULL); + if (argc == 2) { + vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[1]); + return vshCommandStringParse(ctl, argv[1], NULL); } else { - return vshCommandArgvParse(ctl, argc - optind, argv + optind); + return vshCommandArgvParse(ctl, argc - 1, argv + 1); } } return true; } + static const vshCmdDef vshAdmCmds[] = { VSH_CMD_CD, VSH_CMD_ECHO, -- 2.21.0

The GOptionContext API has the benefit over getopt_long that it will automatically handle --help output formatting. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virt-host-validate.c | 86 +++++++++++++++----------------------- 1 file changed, 34 insertions(+), 52 deletions(-) diff --git a/tools/virt-host-validate.c b/tools/virt-host-validate.c index e797e63475..4685e67d0b 100644 --- a/tools/virt-host-validate.c +++ b/tools/virt-host-validate.c @@ -24,7 +24,6 @@ #ifdef HAVE_LIBINTL_H # include <libintl.h> #endif /* HAVE_LIBINTL_H */ -#include <getopt.h> #include "internal.h" #include "virgettext.h" @@ -40,81 +39,64 @@ # include "virt-host-validate-bhyve.h" #endif -static void -show_help(FILE *out, const char *argv0) -{ - fprintf(out, - _("\n" - "syntax: %s [OPTIONS] [HVTYPE]\n" - "\n" - " Hypervisor types:\n" - "\n" - " - qemu\n" - " - lxc\n" - " - bhyve\n" - "\n" - " Options:\n" - " -h, --help Display command line help\n" - " -v, --version Display command version\n" - " -q, --quiet Don't display progress information\n" - "\n"), - argv0); -} - static void show_version(FILE *out, const char *argv0) { fprintf(out, "version: %s %s\n", argv0, VERSION); } -static const struct option argOptions[] = { - { "help", 0, NULL, 'h', }, - { "version", 0, NULL, 'v', }, - { "quiet", 0, NULL, 'q', }, - { NULL, 0, NULL, '\0', } -}; int main(int argc, char **argv) { const char *hvname = NULL; - int c; int ret = EXIT_SUCCESS; bool quiet = false; bool usedHvname = false; + bool version = false; + GOptionEntry opt[] = { + { "version", 'v', 0, + G_OPTION_ARG_NONE, &version, + _("Print version"), NULL }, + { "quiet", 'q', 0, + G_OPTION_ARG_NONE, &quiet, + _("Don't display progress information"), NULL }, + { NULL, 0, 0, 0, NULL, NULL, NULL }, + }; + g_autoptr(GOptionContext) optctx = NULL; + g_autoptr(GError) error = NULL; if (virGettextInitialize() < 0) return EXIT_FAILURE; - while ((c = getopt_long(argc, argv, "hvq", argOptions, NULL)) != -1) { - switch (c) { - case 'v': - show_version(stdout, argv[0]); - return EXIT_SUCCESS; - - case 'h': - show_help(stdout, argv[0]); - return EXIT_SUCCESS; - - case 'q': - quiet = true; - break; - - case '?': - default: - show_help(stderr, argv[0]); - return EXIT_FAILURE; - } + optctx = g_option_context_new(_("HV-TYPE - validate host OS suppport")); + g_option_context_add_main_entries(optctx, opt, PACKAGE); + g_option_context_set_description(optctx, + "Hypervisor types:\n" + "\n" + " - qemu\n" + " - lxc\n" + " - bhyve\n"); + + if (!g_option_context_parse(optctx, &argc, &argv, &error)) { + fprintf(stderr, _("%s: option parsing failed: %s\n"), argv[0], error->message); + return ret; + } + + if (version) { + show_version(stdout, argv[0]); + return EXIT_SUCCESS; } - if ((argc-optind) > 2) { + if (argc > 2) { fprintf(stderr, _("%s: too many command line arguments\n"), argv[0]); - show_help(stderr, argv[0]); + g_autofree char *help = g_option_context_get_help(optctx, TRUE, NULL); + fprintf(stderr, "%s", help); return EXIT_FAILURE; } - if (argc > 1) - hvname = argv[optind]; + if (argc == 2) + hvname = argv[1]; virHostMsgSetQuiet(quiet); -- 2.21.0

The usleep function was missing on older mingw versions, but we can rely on it existing everywhere these days. It may only support times upto 1 second in duration though, so we'll prefer to use g_usleep instead. The commandhelper program is not changed since that can't link to glib. Fortunately it doesn't need to build on Windows platforms either. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- bootstrap.conf | 1 - src/hyperv/hyperv_driver.c | 2 +- src/hyperv/hyperv_wmi.c | 4 ++-- src/locking/lock_daemon.c | 2 +- src/locking/lock_driver_sanlock.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 2 +- src/network/bridge_driver.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 4 ++-- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_tpm.c | 2 +- src/rpc/virnetsocket.c | 2 +- src/security/security_manager.c | 2 +- src/storage/storage_util.c | 4 ++-- src/util/vircgroup.c | 2 +- src/util/virfile.c | 2 +- src/util/virnetdev.c | 2 +- src/util/virnetdevip.c | 2 +- src/util/virnetdevmacvlan.c | 2 +- src/util/virnetdevvportprofile.c | 2 +- src/util/virpci.c | 8 ++++---- src/util/virprocess.c | 4 ++-- src/util/virtime.c | 2 +- src/vbox/vbox_common.c | 2 +- tests/commandtest.c | 6 +++--- tests/eventtest.c | 4 ++-- tests/fdstreamtest.c | 4 ++-- tools/virsh-domain.c | 2 +- 31 files changed, 41 insertions(+), 42 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index ee99889008..d8bef4c995 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -93,7 +93,6 @@ ttyname_r uname unsetenv useless-if-before-free -usleep verify vc-list-files vsnprintf diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 0e2c6c55ef..ceaf528dd3 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -1415,7 +1415,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int codeset, /* simulate holdtime by sleeping */ if (holdtime > 0) - usleep(holdtime * 1000); + g_usleep(holdtime * 1000); /* release the keys */ for (i = 0; i < nkeycodes; i++) { diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index 0f39bd4431..c2c1f082e1 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -909,7 +909,7 @@ hypervInvokeMethod(hypervPrivate *priv, hypervInvokeParamsListPtr params, case MSVM_CONCRETEJOB_JOBSTATE_SHUTTING_DOWN: hypervFreeObject(priv, (hypervObject *)job); job = NULL; - usleep(100 * 1000); /* sleep 100 ms */ + g_usleep(100 * 1000); /* sleep 100 ms */ timeout -= 100; continue; case MSVM_CONCRETEJOB_JOBSTATE_COMPLETED: @@ -1418,7 +1418,7 @@ hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain, hypervFreeObject(priv, (hypervObject *)concreteJob); concreteJob = NULL; - usleep(100 * 1000); + g_usleep(100 * 1000); continue; case MSVM_CONCRETEJOB_JOBSTATE_COMPLETED: diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index a5a3a97e99..c12cb4ea0f 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -657,7 +657,7 @@ virLockDaemonClientFree(void *opaque) VIR_WARN("Failed to kill off pid %lld", (unsigned long long)priv->clientPid); } - usleep(200 * 1000); + g_usleep(200 * 1000); } } } diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 85a23c7642..7ebd63913e 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -376,7 +376,7 @@ virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver) #else /* fall back to polling */ VIR_DEBUG("Sleeping for %dms", LOCKSPACE_SLEEP); - usleep(LOCKSPACE_SLEEP * 1000); + g_usleep(LOCKSPACE_SLEEP * 1000); #endif VIR_DEBUG("Retrying to add lockspace (left %d)", retries); goto retry; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 37851bf284..9097655b4d 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -574,7 +574,7 @@ static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl, while (!virFileExists(pidpath)) { /* wait for 100ms before checking again, but don't do it for ever */ if (errno == ENOENT && loops < 10) { - usleep(100 * 1000); + g_usleep(100 * 1000); loops++; } else { virReportSystemError(errno, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index cf1dd1428e..5cc6b2533d 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2960,7 +2960,7 @@ static int lxcFreezeContainer(virDomainObjPtr vm) * decide that the freezing has been complete only with * the state actually transit to "FROZEN". */ - usleep(check_interval * 1000); + g_usleep(check_interval * 1000); r = virCgroupGetFreezerState(priv->cgroup, &state); diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index cbdc7b1268..318b4c1653 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1095,7 +1095,7 @@ virLXCProcessReadLogOutputData(virDomainObjPtr vm, goto cleanup; } - usleep(100*1000); + g_usleep(100*1000); retries--; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ae2e4f09f8..3be9d59bc0 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1028,7 +1028,7 @@ networkKillDaemon(pid_t pid, * than modifications to domains, this seems a reasonable * tradeoff in exchange for less code disruption. */ - usleep(20 * 1000); + g_usleep(20 * 1000); } VIR_WARN("Timed out waiting after SIG%s to %s process %d " "(network '%s')", diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 2af8cfdd32..c82457989e 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1273,7 +1273,7 @@ virNWFilterSnoopRatePenalty(virNWFilterSnoopPcapConfPtr pc, unsigned long long now; if (virTimeMillisNowRaw(&now) < 0) { - usleep(PCAP_FLOOD_TIMEOUT_MS); /* 1 ms */ + g_usleep(PCAP_FLOOD_TIMEOUT_MS); /* 1 ms */ pc->penaltyTimeoutAbs = 0; } else { /* don't listen to the fd for 1 ms */ @@ -2010,7 +2010,7 @@ virNWFilterSnoopJoinThreads(void) while (virAtomicIntGet(&virNWFilterSnoopState.nThreads) != 0) { VIR_WARN("Waiting for snooping threads to terminate: %u", virAtomicIntGet(&virNWFilterSnoopState.nThreads)); - usleep(1000 * 1000); + g_usleep(1000 * 1000); } } diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c index cd66e3ea7d..34b8d7fcd9 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -812,7 +812,7 @@ virNWFilterLearnThreadsTerminate(bool allowNewThreads) threadsTerminate = true; while (virHashSize(pendingLearnReq) != 0) - usleep((PKT_TIMEOUT_MS * 1000) / 3); + g_usleep((PKT_TIMEOUT_MS * 1000) / 3); if (allowNewThreads) threadsTerminate = false; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a6facdc09b..dbc32acb5e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1650,7 +1650,7 @@ qemuMonitorJSONStartCPUs(qemuMonitorPtr mon) virJSONValueFree(reply); reply = NULL; - usleep(250000); + g_usleep(250000); } while (++i <= timeout); virJSONValueFree(cmd); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 872750ff4d..2aa1c71349 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7622,7 +7622,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, retry: if ((ret = qemuRemoveCgroup(vm)) < 0) { if (ret == -EBUSY && (retries++ < 5)) { - usleep(200*1000); + g_usleep(200*1000); goto retry; } VIR_WARN("Failed to remove cgroup for %s", diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 1d68803a28..31fdfa995f 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -884,7 +884,7 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); if (rc < 0) { timeout -= 50; - usleep(50 * 1000); + g_usleep(50 * 1000); continue; } if (rc == 0 && pid == (pid_t)-1) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a469907779..bd5fa96751 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -749,7 +749,7 @@ int virNetSocketNewConnectUNIX(const char *path, daemonLaunched = true; } - usleep(10000); + g_usleep(10000); } localAddr.len = sizeof(localAddr.data); diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 7c905f0785..7f187c9068 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1349,7 +1349,7 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if (retries && (errno == EACCES || errno == EAGAIN)) { /* File is locked. Try again. */ retries--; - usleep(1000); + g_usleep(1000); continue; } else { virReportSystemError(errno, diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 6165dd43b2..be084119f4 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1940,7 +1940,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, if (virDirOpenQuiet(&dh, def->target.path) < 0) { opentries++; if (loop && errno == ENOENT && opentries < 50) { - usleep(100 * 1000); + g_usleep(100 * 1000); goto reopen; } virReportSystemError(errno, @@ -1975,7 +1975,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, } if (!direrr && loop && ++retry < 100) { - usleep(100 * 1000); + g_usleep(100 * 1000); goto retry; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 4f9d80666d..70d41c4ba5 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2656,7 +2656,7 @@ virCgroupKillPainfully(virCgroupPtr group) if (ret <= 0) break; - usleep(200 * 1000); + g_usleep(200 * 1000); } VIR_DEBUG("Complete %d", ret); return ret; diff --git a/src/util/virfile.c b/src/util/virfile.c index bb844c64e5..dead335c62 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4387,7 +4387,7 @@ virFileWaitForExists(const char *path, if (tries == 0 || errno != ENOENT) return -1; - usleep(ms * 1000); + g_usleep(ms * 1000); } return 0; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5c0e9723b9..5fef0b79aa 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2304,7 +2304,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf, * wait, then upcoming operations on the VF may fail. */ while (retries-- > 0 && !virNetDevExists(linkdev)) - usleep(1000); + g_usleep(1000); } if (pfDevOrig && setMACrc == 0) { diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c index e2009fd829..e9d1ce0831 100644 --- a/src/util/virnetdevip.c +++ b/src/util/virnetdevip.c @@ -465,7 +465,7 @@ virNetDevIPWaitDadFinish(virSocketAddrPtr *addrs, size_t count) /* Parse response. */ dad = virNetDevIPParseDadStatus(resp, recvbuflen, addrs, count); if (dad) - usleep(1000 * 10); + g_usleep(1000 * 10); } /* Check timeout. */ if (dad) { diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index e8a9b052b6..efd81c3aa0 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -404,7 +404,7 @@ virNetDevMacVLanTapOpen(const char *ifname, tapfd[i] = fd; } else if (retries-- > 0) { /* may need to wait for udev to be done */ - usleep(20000); + g_usleep(20000); } else { /* However, if haven't succeeded, quit. */ virReportSystemError(errno, diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index a5ecd783f2..048f891920 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -999,7 +999,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, break; } - usleep(STATUS_POLL_INTERVL_USEC); + g_usleep(STATUS_POLL_INTERVL_USEC); } if (status == PORT_PROFILE_RESPONSE_INPROGRESS) { diff --git a/src/util/virpci.c b/src/util/virpci.c index ee78151e74..9b828b7781 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -833,11 +833,11 @@ virPCIDeviceTrySecondaryBusReset(virPCIDevicePtr dev, virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, ctl | PCI_BRIDGE_CTL_RESET); - usleep(200 * 1000); /* sleep 200ms */ + g_usleep(200 * 1000); /* sleep 200ms */ virPCIDeviceWrite16(parent, parentfd, PCI_BRIDGE_CONTROL, ctl); - usleep(200 * 1000); /* sleep 200ms */ + g_usleep(200 * 1000); /* sleep 200ms */ if (virPCIDeviceWrite(dev, cfgfd, 0, config_space, PCI_CONF_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -881,12 +881,12 @@ virPCIDeviceTryPowerManagementReset(virPCIDevicePtr dev, int cfgfd) virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, ctl | PCI_PM_CTRL_STATE_D3hot); - usleep(10 * 1000); /* sleep 10ms */ + g_usleep(10 * 1000); /* sleep 10ms */ virPCIDeviceWrite32(dev, cfgfd, dev->pci_pm_cap_pos + PCI_PM_CTRL, ctl | PCI_PM_CTRL_STATE_D0); - usleep(10 * 1000); /* sleep 10ms */ + g_usleep(10 * 1000); /* sleep 10ms */ if (virPCIDeviceWrite(dev, cfgfd, 0, &config_space[0], PCI_CONF_LEN) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 66834d37d3..b1544af730 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -177,7 +177,7 @@ virProcessAbort(pid_t pid) } else if (ret == 0) { VIR_DEBUG("trying SIGTERM to child process %d", pid); kill(pid, SIGTERM); - usleep(10 * 1000); + g_usleep(10 * 1000); while ((ret = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR); if (ret == pid) { @@ -399,7 +399,7 @@ virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int extradelay) goto cleanup; /* process is dead */ } - usleep(200 * 1000); + g_usleep(200 * 1000); } virReportSystemError(EBUSY, diff --git a/src/util/virtime.c b/src/util/virtime.c index 86993e4e8d..6f978d7c64 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -448,6 +448,6 @@ virTimeBackOffWait(virTimeBackOffVar *var) VIR_DEBUG("sleeping for %llu ms", next); - usleep(next * 1000); + g_usleep(next * 1000); return 1; } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index d3b8fb625f..748133933c 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -7932,7 +7932,7 @@ vboxDomainSendKey(virDomainPtr dom, /* since VBOX does not support holdtime, simulate it by sleeping and then sending the release key scancodes */ if (holdtime > 0) - usleep(holdtime * 1000); + g_usleep(holdtime * 1000); rc = gVBoxAPI.UIKeyboard.PutScancodes(keyboard, nkeycodes, keyUpCodes, &codesStored); diff --git a/tests/commandtest.c b/tests/commandtest.c index 2aaddef3d1..2ec3434f1e 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -262,7 +262,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) goto cleanup; } while (kill(pid, 0) != -1) - usleep(100*1000); + g_usleep(100*1000); ret = checkoutput("test4", NULL); @@ -751,7 +751,7 @@ static int test18(const void *unused ATTRIBUTE_UNUSED) } while (kill(pid, SIGINT) != -1) - usleep(100*1000); + g_usleep(100*1000); ret = 0; @@ -1052,7 +1052,7 @@ static int test25(const void *unused ATTRIBUTE_UNUSED) goto cleanup; } - usleep(10 * 1000); + g_usleep(10 * 1000); } else { break; } diff --git a/tests/eventtest.c b/tests/eventtest.c index f426469d21..f04b416ad2 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -382,7 +382,7 @@ mymain(void) startJob(); pthread_mutex_unlock(&eventThreadMutex); sched_yield(); - usleep(100 * 1000); + g_usleep(100 * 1000); pthread_mutex_lock(&eventThreadMutex); virEventPollRemoveHandle(handles[1].watch); if (finishJob("Interrupted during poll", -1, -1) != EXIT_SUCCESS) @@ -448,7 +448,7 @@ mymain(void) startJob(); pthread_mutex_unlock(&eventThreadMutex); sched_yield(); - usleep(100 * 1000); + g_usleep(100 * 1000); pthread_mutex_lock(&eventThreadMutex); virEventPollRemoveTimeout(timers[1].timer); if (finishJob("Interrupted during poll", -1, -1) != EXIT_SUCCESS) diff --git a/tests/fdstreamtest.c b/tests/fdstreamtest.c index 054c405cec..98519e6266 100644 --- a/tests/fdstreamtest.c +++ b/tests/fdstreamtest.c @@ -101,7 +101,7 @@ static int testFDStreamReadCommon(const char *scratchdir, bool blocking) got = st->driver->streamRecv(st, buf + offset, want); if (got < 0) { if (got == -2 && !blocking) { - usleep(20 * 1000); + g_usleep(20 * 1000); goto reread; } virFilePrintf(stderr, "Failed to read stream: %s\n", @@ -222,7 +222,7 @@ static int testFDStreamWriteCommon(const char *scratchdir, bool blocking) got = st->driver->streamSend(st, pattern + offset, want); if (got < 0) { if (got == -2 && !blocking) { - usleep(20 * 1000); + g_usleep(20 * 1000); goto rewrite; } if (i == 9 && diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fbfdc09c0d..c2e7c2b227 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1945,7 +1945,7 @@ virshBlockJobWait(virshBlockJobWaitDataPtr data) break; } - usleep(500 * 1000); + g_usleep(500 * 1000); } /* print 100% completed */ -- 2.21.0