[libvirt] [PATCH 0/5] examples: enforce compiler warning flags
The example programs emitted a number of warnings even without having the WARN_CFLAGS set. Once they were set a bunch more flaws came to light. Fix them all and enforce warnings so that the example programs are higher quality code. Daniel P. Berrangé (5): examples: fix 64-bit integer formatting on Windows examples: avoid goto jump over initialization of variable domtop: remove unused domain name parameter dominfo: make example more useful examples: enable all compiler warnings examples/Makefile.am | 3 ++- examples/admin/client_info.c | 7 ++++--- examples/admin/client_limits.c | 2 +- examples/admin/list_clients.c | 3 ++- examples/admin/threadpool_params.c | 2 +- examples/dominfo/info1.c | 25 ++++++++++++++----------- examples/domtop/domtop.c | 13 ++++++++----- 7 files changed, 32 insertions(+), 23 deletions(-) -- 2.20.1
The Windows printf functions don't support %llu/%lld for printing 64-bit integers. For most of libvirt this doesn't matter as we rely on gnulib which provides a replacement printf that is sane. The example code is designed to compile against the normal OS headers, with no use of gnulib and thus has to use the platform specific printf. To deal with this we must use the macros PRI* macros from inttypes.h to get the platform specific format string. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- examples/admin/client_info.c | 7 ++++--- examples/admin/list_clients.c | 3 ++- examples/domtop/domtop.c | 8 ++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/examples/admin/client_info.c b/examples/admin/client_info.c index f3f62a656b..7fc6c72bbd 100644 --- a/examples/admin/client_info.c +++ b/examples/admin/client_info.c @@ -3,6 +3,7 @@ #include <stdlib.h> #include <time.h> #include <string.h> +#include <inttypes.h> #include <libvirt/libvirt-admin.h> static const char * @@ -66,11 +67,11 @@ exampleGetTypedParamValue(virTypedParameterPtr item) break; case VIR_TYPED_PARAM_LLONG: - ret = asprintf(&str, "%lld", item->value.l); + ret = asprintf(&str, "%" PRId64, (int64_t)item->value.l); break; case VIR_TYPED_PARAM_ULLONG: - ret = asprintf(&str, "%llu", item->value.ul); + ret = asprintf(&str, "%" PRIu64, (uint64_t)item->value.ul); break; case VIR_TYPED_PARAM_DOUBLE: @@ -143,7 +144,7 @@ int main(int argc, char **argv) if (!(timestr = exampleGetTimeStr(virAdmClientGetTimestamp(clnt)))) goto cleanup; - printf("%-15s: %llu\n", "id", virAdmClientGetID(clnt)); + printf("%-15s: %" PRIu64 "\n", "id", (uint64_t)virAdmClientGetID(clnt)); printf("%-15s: %s\n", "connection_time", timestr); printf("%-15s: %s\n", "transport", exampleTransportToString(virAdmClientGetTransport(clnt))); diff --git a/examples/admin/list_clients.c b/examples/admin/list_clients.c index 5cf8e4c2a6..2876637d42 100644 --- a/examples/admin/list_clients.c +++ b/examples/admin/list_clients.c @@ -1,6 +1,7 @@ #include <stdio.h> #include <stdlib.h> #include <time.h> +#include <inttypes.h> #include <libvirt/libvirt-admin.h> static const char * @@ -96,7 +97,7 @@ int main(int argc, char **argv) exampleGetTimeStr(virAdmClientGetTimestamp(client)))) goto cleanup; - printf(" %-5llu %-15s %-15s\n", id, + printf(" %-5" PRIu64 " %-15s %-15s\n", (uint64_t)id, exampleTransportToString(transport), timestr); free(timestr); } diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c index 008065c651..e1e7fbff8b 100644 --- a/examples/domtop/domtop.c +++ b/examples/domtop/domtop.c @@ -29,6 +29,7 @@ #include <string.h> #include <sys/time.h> #include <unistd.h> +#include <inttypes.h> static bool debug; static bool run_top; @@ -226,8 +227,11 @@ print_cpu_usage(const char *dom_name, return; } - DEBUG("now_params=%llu then_params=%llu now=%llu then=%llu", - now_params[pos].value.ul, then_params[pos].value.ul, now, then); + DEBUG("now_params=%" PRIu64 " then_params=%" PRIu64 + " now=%" PRIu64 " then=%" PRIu64, + (uint64_t)now_params[pos].value.ul, + (uint64_t)then_params[pos].value.ul, + (uint64_t)now, (uint64_t)then); /* @now_params and @then_params are in nanoseconds, @now and @then are * in microseconds. In ideal world, we would translate them both into -- 2.20.1
Jumping over the declaration and initialization of a variable is bad as it means the jump target sees a potentially non-initialized variable. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- examples/admin/client_limits.c | 2 +- examples/admin/threadpool_params.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/admin/client_limits.c b/examples/admin/client_limits.c index 69b931682c..fb29ee6cbf 100644 --- a/examples/admin/client_limits.c +++ b/examples/admin/client_limits.c @@ -9,6 +9,7 @@ int main(int argc, char **argv) virAdmServerPtr srv = NULL; /* which server to work with */ virTypedParameterPtr params = NULL; int nparams = 0; + int maxparams = 0; ssize_t i; if (argc != 2) { @@ -39,7 +40,6 @@ int main(int argc, char **argv) nparams = 0; /* set nclients_max to 100 and nclients_unauth_max to 20 */ - int maxparams = 0; if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, VIR_SERVER_CLIENTS_MAX, 100) < 0 || virTypedParamsAddUInt(¶ms, &nparams, &maxparams, diff --git a/examples/admin/threadpool_params.c b/examples/admin/threadpool_params.c index be833c2ea3..8e6354576f 100644 --- a/examples/admin/threadpool_params.c +++ b/examples/admin/threadpool_params.c @@ -9,6 +9,7 @@ int main(int argc, char **argv) virAdmServerPtr srv = NULL; /* which server to work with */ virTypedParameterPtr params = NULL; int nparams = 0; + int maxparams = 0; ssize_t i; if (argc != 2) { @@ -39,7 +40,6 @@ int main(int argc, char **argv) nparams = 0; /* let's set minWorkers to 10, maxWorkers to 15 and prioWorkers to 10 */ - int maxparams = 0; if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, VIR_THREADPOOL_WORKERS_MIN, 10) < 0 || virTypedParamsAddUInt(¶ms, &nparams, &maxparams, -- 2.20.1
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- examples/domtop/domtop.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c index e1e7fbff8b..c9b51aed51 100644 --- a/examples/domtop/domtop.c +++ b/examples/domtop/domtop.c @@ -186,8 +186,7 @@ fetch_domains(virConnectPtr conn) } static void -print_cpu_usage(const char *dom_name, - size_t cpu, +print_cpu_usage(size_t cpu, size_t ncpus, unsigned long long then, virTypedParameterPtr then_params, @@ -333,7 +332,7 @@ do_top(virConnectPtr conn, goto cleanup; } - print_cpu_usage(dom_name, 0, max_id, + print_cpu_usage(0, max_id, then.tv_sec * 1000000 + then.tv_usec, then_params, then_nparams, now.tv_sec * 1000000 + now.tv_usec, -- 2.20.1
The example currently assumes that a NULL URI will open Xen and thus also assumes that a domain with ID 0 exists. Change it to require the URI and a domain name as command line arguments. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- examples/dominfo/info1.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/examples/dominfo/info1.c b/examples/dominfo/info1.c index 09f20358f1..f0ea9b9f08 100644 --- a/examples/dominfo/info1.c +++ b/examples/dominfo/info1.c @@ -13,40 +13,39 @@ /** * getDomainInfo: - * @id: the id of the domain + * @name: the name of the domain * * extract the domain 0 information */ static void -getDomainInfo(int id) +getDomainInfo(const char *uri, const char *name) { virConnectPtr conn = NULL; /* the hypervisor connection */ virDomainPtr dom = NULL; /* the domain being checked */ virDomainInfo info; /* the information being fetched */ int ret; - /* NULL means connect to local Xen hypervisor */ - conn = virConnectOpenReadOnly(NULL); + conn = virConnectOpenReadOnly(uri); if (conn == NULL) { fprintf(stderr, "Failed to connect to hypervisor\n"); goto error; } - /* Find the domain of the given id */ - dom = virDomainLookupByID(conn, id); + /* Find the domain of the given name */ + dom = virDomainLookupByName(conn, name); if (dom == NULL) { - fprintf(stderr, "Failed to find Domain %d\n", id); + fprintf(stderr, "Failed to find Domain %s\n", name); goto error; } /* Get the information */ ret = virDomainGetInfo(dom, &info); if (ret < 0) { - fprintf(stderr, "Failed to get information for Domain %d\n", id); + fprintf(stderr, "Failed to get information for Domain %s\n", name); goto error; } - printf("Domains %d: %d CPUs\n", id, info.nrVirtCpu); + printf("Domain %s: %d CPUs\n", name, info.nrVirtCpu); error: if (dom != NULL) @@ -55,9 +54,13 @@ getDomainInfo(int id) virConnectClose(conn); } -int main() +int main(int argc, char **argv) { - getDomainInfo(0); + if (argc != 3) { + fprintf(stderr, "syntax: %s: URI NAME\n", argv[0]); + return 1; + } + getDomainInfo(argv[1], argv[2]); return 0; } -- 2.20.1
Now that all the examples are warning free, keep it that way by enabling all the normal compiler warning flags. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- examples/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/Makefile.am b/examples/Makefile.am index b590a148ce..ee7d3e6b2c 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -28,7 +28,8 @@ EXTRA_DIST = \ AM_CPPFLAGS = \ - -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir) + -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir) \ + $(WARN_CFLAGS) LDADD = $(STATIC_BINARIES) $(WARN_CFLAGS) \ $(top_builddir)/src/libvirt.la \ $(top_builddir)/src/libvirt-admin.la -- 2.20.1
On Tue, Apr 02, 2019 at 11:02:06AM +0100, Daniel P. Berrangé wrote:
The example programs emitted a number of warnings even without having the WARN_CFLAGS set. Once they were set a bunch more flaws came to light. Fix them all and enforce warnings so that the example programs are higher quality code.
Daniel P. Berrangé (5): examples: fix 64-bit integer formatting on Windows examples: avoid goto jump over initialization of variable domtop: remove unused domain name parameter dominfo: make example more useful examples: enable all compiler warnings
examples/Makefile.am | 3 ++- examples/admin/client_info.c | 7 ++++--- examples/admin/client_limits.c | 2 +- examples/admin/list_clients.c | 3 ++- examples/admin/threadpool_params.c | 2 +- examples/dominfo/info1.c | 25 ++++++++++++++----------- examples/domtop/domtop.c | 13 ++++++++----- 7 files changed, 32 insertions(+), 23 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Daniel P. Berrangé -
Ján Tomko