[libvirt] [PATCH 0/2] Remove a broken example code and kill usage of atoi in libvirt
Peter Krempa (2): examples: Remove broken bad example maint: Kill usage of atoi() .gitignore | 1 - Makefile.am | 4 +- cfg.mk | 6 ++ examples/domsuspend/Makefile.am | 25 ------- examples/domsuspend/suspend.c | 136 ------------------------------------- libvirt.spec.in | 3 +- src/conf/storage_conf.h | 2 +- src/storage/storage_backend_disk.c | 14 ++-- src/xen/xend_internal.c | 17 +++-- 9 files changed, 32 insertions(+), 176 deletions(-) delete mode 100644 examples/domsuspend/Makefile.am delete mode 100644 examples/domsuspend/suspend.c -- 1.8.4.3
The domsuspend example code is a really old and bad exmample of (how not to use) the libvirt API. Remove it as it's apparent that nobody tried to use it. It was broken and nobody complained. --- .gitignore | 1 - Makefile.am | 4 +- examples/domsuspend/Makefile.am | 25 -------- examples/domsuspend/suspend.c | 136 ---------------------------------------- libvirt.spec.in | 3 +- 5 files changed, 3 insertions(+), 166 deletions(-) delete mode 100644 examples/domsuspend/Makefile.am delete mode 100644 examples/domsuspend/suspend.c diff --git a/.gitignore b/.gitignore index 8de0b26..24469a1 100644 --- a/.gitignore +++ b/.gitignore @@ -71,7 +71,6 @@ /docs/todo.html.in /examples/domain-events/events-c/event-test /examples/dominfo/info1 -/examples/domsuspend/suspend /examples/hellolibvirt/hellolibvirt /examples/openauth/openauth /gnulib/lib/* diff --git a/Makefile.am b/Makefile.am index d7ddd9d..2cbf71a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -21,8 +21,8 @@ GENHTML = genhtml SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ tests po examples/domain-events/events-c examples/hellolibvirt \ - examples/dominfo examples/domsuspend examples/apparmor \ - examples/xml/nwfilter examples/openauth examples/systemtap + examples/dominfo examples/apparmor examples/xml/nwfilter \ + examples/openauth examples/systemtap ACLOCAL_AMFLAGS = -I m4 diff --git a/examples/domsuspend/Makefile.am b/examples/domsuspend/Makefile.am deleted file mode 100644 index d0d9368..0000000 --- a/examples/domsuspend/Makefile.am +++ /dev/null @@ -1,25 +0,0 @@ -## Copyright (C) 2005-2013 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 -## License as published by the Free Software Foundation; either -## version 2.1 of the License, or (at your option) any later version. -## -## This library is distributed in the hope that it will be useful, -## but WITHOUT ANY WARRANTY; without even the implied warranty of -## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU -## Lesser General Public License for more details. -## -## You should have received a copy of the GNU Lesser General Public -## License along with this library. If not, see -## <http://www.gnu.org/licenses/>. - -INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include -LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \ - $(COVERAGE_LDFLAGS) - -noinst_PROGRAMS=suspend - -suspend_SOURCES=suspend.c -suspend_LDFLAGS= -suspend_LDADD= $(LDADDS) diff --git a/examples/domsuspend/suspend.c b/examples/domsuspend/suspend.c deleted file mode 100644 index 6a0f5c9..0000000 --- a/examples/domsuspend/suspend.c +++ /dev/null @@ -1,136 +0,0 @@ -/** - * section: Scheduling - * synopsis: Suspend a domain and then resume its execution - * purpose: Demonstrate the basic use of the library to suspend and - * resume a domain. If no id is given on the command line - * this script will suspend and resume the first domain found - * which is not Domain 0. - * usage: suspend [id] - * test: suspend - * author: Daniel Veillard - * copy: see Copyright for the status of this software. - */ - -#include <stdlib.h> -#include <stdio.h> -#include <libvirt/libvirt.h> - -static virConnectPtr conn = NULL; /* the hypervisor connection */ - -/** - * checkDomainState: - * @dom: the domain - * - * Return the current state of a domain or -1 if non-exsitant - */ -static int -checkDomainState(virDomainPtr dom) { - virDomainInfo info; /* the information being fetched */ - int ret; - - ret = virDomainGetInfo(dom, &info); - if (ret < 0) { - return -1; - } - return info.state; -} - -/** - * SuspendAndResumeDomain: - * @id: the id of the domain - * - * extract the domain 0 information - */ -static void -SuspendAndResumeDomain(int id) { - virDomainPtr dom = NULL; /* the domain being checked */ - int ret, state; - - /* Find the domain of the given id */ - dom = virDomainLookupByID(conn, id); - if (dom == NULL) { - fprintf(stderr, "Failed to find Domain %d\n", id); - goto error; - } - - /* Check state */ - state = checkDomainState(dom); - if ((state == VIR_DOMAIN_RUNNING) || - (state == VIR_DOMAIN_NOSTATE) || - (state == VIR_DOMAIN_BLOCKED)) { - printf("Suspending domain...\n"); - ret = virDomainSuspend(dom); - if (ret < 0) { - fprintf(stderr, "Failed to suspend Domain %d\n", id); - goto error; - } - state = checkDomainState(dom); - if (state != VIR_DOMAIN_PAUSED) { - fprintf(stderr, "Domain %d state is not suspended\n", id); - } else { - printf("Domain suspended, resuming it...\n"); - } - ret = virDomainResume(dom); - if (ret < 0) { - fprintf(stderr, "Failed to resume Domain %d\n", id); - goto error; - } - state = checkDomainState(dom); - if ((state == VIR_DOMAIN_RUNNING) || - (state == VIR_DOMAIN_NOSTATE) || - (state == VIR_DOMAIN_BLOCKED)) { - printf("Domain resumed\n"); - } else { - fprintf(stderr, "Domain %d state indicate it is not resumed\n", id); - } - } else { - fprintf(stderr, "Domain %d is not in a state where it should be suspended\n", id); - goto error; - } - -error: - if (dom != NULL) - virDomainFree(dom); -} - -int main(int argc, char **argv) { - int id = 0; - - /* NULL means connect to local Xen hypervisor */ - conn = virConnectOpenReadOnly(NULL); - if (conn == NULL) { - fprintf(stderr, "Failed to connect to hypervisor\n"); - goto error; - } - - if (argc > 1) { - id = atoi(argv[1]); - } - if (id == 0) { - int n; - size_t i; - int ids[10]; - n = virConnectListDomains(conn, &ids[0], 10); - if (n < 0) { - fprintf(stderr, "Failed to list the domains\n"); - goto error; - } - for (i = 0; i < n; i++) { - if (ids[i] != 0) { - id = ids[i]; - break; - } - } - } - if (id == 0) { - fprintf(stderr, "Failed find a running guest domain\n"); - goto error; - } - - SuspendAndResumeDomain(id); - -error: - if (conn != NULL) - virConnectClose(conn); - return 0; -} diff --git a/libvirt.spec.in b/libvirt.spec.in index cfd8890..eff7103 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1428,7 +1428,7 @@ rm -fr %{buildroot} # on RHEL 5, thus we need to expand it here. make install DESTDIR=%{?buildroot} SYSTEMD_UNIT_DIR=%{_unitdir} -for i in domain-events/events-c dominfo domsuspend hellolibvirt openauth xml/nwfilter systemtap +for i in domain-events/events-c dominfo hellolibvirt openauth xml/nwfilter systemtap do (cd examples/$i ; make clean ; rm -rf .deps .libs Makefile Makefile.in) done @@ -2094,7 +2094,6 @@ exit 0 %doc examples/hellolibvirt %doc examples/domain-events/events-c %doc examples/dominfo -%doc examples/domsuspend %doc examples/openauth %doc examples/xml %doc examples/systemtap -- 1.8.4.3
Kill the use of atoi() and introduce syntax check to forbid it and it's friends (atol, atoll, atof, atoq). Also fix a typo in variable name holding the cylinders count of a disk pool (apparently unused). examples/domsuspend/suspend.c will need a larger scale refactor as the whole example file is broken thus it will be exempted from the syntax check for now. --- cfg.mk | 6 ++++++ src/conf/storage_conf.h | 2 +- src/storage/storage_backend_disk.c | 14 +++++++++----- src/xen/xend_internal.c | 17 +++++++++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/cfg.mk b/cfg.mk index bd3dd48..0c57022 100644 --- a/cfg.mk +++ b/cfg.mk @@ -869,6 +869,12 @@ sc_prohibit_getenv: halt='Use virGetEnv{Allow,Block}SUID instead of getenv' \ $(_sc_search_regexp) +sc_prohibit_atoi: + @prohibit='\bato(i|f|l|ll|q) *\(' \ + halt='Use virStrToLong* instead of atoi, atol, atof, atoq, atoll' \ + $(_sc_search_regexp) + + # We don't use this feature of maint.mk. prev_version_file = /dev/null diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index f8a7eec..485bdba 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -229,7 +229,7 @@ struct _virStoragePoolSourceDevice { * the geometry data is needed */ struct _geometry { - int cyliders; + int cylinders; int heads; int sectors; } geometry; diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4e53ec5..a7a7d0e 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -280,12 +280,16 @@ virStorageBackendDiskMakePoolGeometry(virStoragePoolObjPtr pool, char **const groups, void *data ATTRIBUTE_UNUSED) { + virStoragePoolSourceDevicePtr device = &(pool->def->source.devices[0]); + if (virStrToLong_i(groups[0], NULL, 0, &device->geometry.cylinders) < 0 || + virStrToLong_i(groups[1], NULL, 0, &device->geometry.heads) < 0 || + virStrToLong_i(groups[2], NULL, 0, &device->geometry.sectors) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to create disk pool geometry")); + return -1; + } - pool->def->source.devices[0].geometry.cyliders = atoi(groups[0]); - pool->def->source.devices[0].geometry.heads = atoi(groups[1]); - pool->def->source.devices[0].geometry.sectors = atoi(groups[2]); - - return 0; + return 0; } static int diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index dac9a79..87e77a6 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -290,10 +290,19 @@ xend_req(int fd, char **content) if (STREQ(buffer, "\r\n")) break; - if (istartswith(buffer, "Content-Length: ")) - content_length = atoi(buffer + 16); - else if (istartswith(buffer, "HTTP/1.1 ")) - retcode = atoi(buffer + 9); + if (istartswith(buffer, "Content-Length: ")) { + if (virStrToLong_i(buffer + 16, NULL, 10, &content_length) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse Xend response content length")); + return -1; + } + } else if (istartswith(buffer, "HTTP/1.1 ")) { + if (virStrToLong_i(buffer + 9, NULL, 10, &retcode) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse Xend response return code")); + return -1; + } + } } VIR_FREE(buffer); -- 1.8.4.3
On 04.12.2013 11:31, Peter Krempa wrote:
Peter Krempa (2): examples: Remove broken bad example maint: Kill usage of atoi()
.gitignore | 1 - Makefile.am | 4 +- cfg.mk | 6 ++ examples/domsuspend/Makefile.am | 25 ------- examples/domsuspend/suspend.c | 136 ------------------------------------- libvirt.spec.in | 3 +- src/conf/storage_conf.h | 2 +- src/storage/storage_backend_disk.c | 14 ++-- src/xen/xend_internal.c | 17 +++-- 9 files changed, 32 insertions(+), 176 deletions(-) delete mode 100644 examples/domsuspend/Makefile.am delete mode 100644 examples/domsuspend/suspend.c
ACK series Michal
On 12/04/13 11:51, Michal Privoznik wrote:
On 04.12.2013 11:31, Peter Krempa wrote:
Peter Krempa (2): examples: Remove broken bad example maint: Kill usage of atoi()
.gitignore | 1 - Makefile.am | 4 +- cfg.mk | 6 ++ examples/domsuspend/Makefile.am | 25 ------- examples/domsuspend/suspend.c | 136 ------------------------------------- libvirt.spec.in | 3 +- src/conf/storage_conf.h | 2 +- src/storage/storage_backend_disk.c | 14 ++-- src/xen/xend_internal.c | 17 +++-- 9 files changed, 32 insertions(+), 176 deletions(-) delete mode 100644 examples/domsuspend/Makefile.am delete mode 100644 examples/domsuspend/suspend.c
ACK series
Thanks for the review. As nobody objected up until now, I've pushed this series. Peter
participants (2)
-
Michal Privoznik -
Peter Krempa