[libvirt] [PATCH] maint: Kill usage of atoi()

Also fix a typo in variable name holding the cylinders count of a disk pool (apparently unused). --- cfg.mk | 10 ++++++++++ src/conf/storage_conf.h | 2 +- src/qemu/qemu_command.c | 8 ++++++-- src/storage/storage_backend_disk.c | 16 +++++++++++----- src/xen/xend_internal.c | 17 +++++++++++++---- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/cfg.mk b/cfg.mk index befd231..4df81d2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -869,6 +869,13 @@ sc_prohibit_getenv: halt='Use virGetEnv{Allow,Block}SUID instead of getenv' \ $(_sc_search_regexp) +sc_prohibit_atoi: + @prohibit='\batoi *\(' \ + exclude='exempt from syntax-check' \ + halt='Use virStrToLong* instead of atoi' \ + $(_sc_search_regexp) + + # We don't use this feature of maint.mk. prev_version_file = /dev/null @@ -1040,3 +1047,6 @@ exclude_file_name_regexp--sc_prohibit_int_ijk = \ exclude_file_name_regexp--sc_prohibit_getenv = \ ^tests/.*\.[ch]$$ + +exclude_file_name_regexp--sc_prohibit_atoi= \ + ^examples/.*\.[ch]$$ diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index f062bd8..05c291e 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -227,7 +227,7 @@ struct _virStoragePoolSourceDevice { * the geometry data is needed */ struct _geometry { - int cyliders; + int cylinders; int heads; int sectors; } geometry; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 966aa0d..25beedf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3663,8 +3663,12 @@ qemuBuildDriveURIString(virConnectPtr conn, if (disk->src && virAsprintf(&volimg, "/%s", disk->src) < 0) goto cleanup; - if (disk->hosts->port) { - port = atoi(disk->hosts->port); + if (disk->hosts->port && + virStrToLong_i(disk->hosts->port, NULL, 0, &port) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse network port '%s'"), + disk->hosts->port); + goto cleanup; } if (disk->hosts->socket && diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4e53ec5..1f30b06 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -280,12 +280,18 @@ virStorageBackendDiskMakePoolGeometry(virStoragePoolObjPtr pool, char **const groups, void *data ATTRIBUTE_UNUSED) { + pool = pool; + groups = groups; + 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 dc57350..771288c 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 11/14/13 17:20, Peter Krempa wrote:
Also fix a typo in variable name holding the cylinders count of a disk pool (apparently unused). --- cfg.mk | 10 ++++++++++ src/conf/storage_conf.h | 2 +- src/qemu/qemu_command.c | 8 ++++++-- src/storage/storage_backend_disk.c | 16 +++++++++++----- src/xen/xend_internal.c | 17 +++++++++++++---- 5 files changed, 41 insertions(+), 12 deletions(-)
...
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4e53ec5..1f30b06 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -280,12 +280,18 @@ virStorageBackendDiskMakePoolGeometry(virStoragePoolObjPtr pool, char **const groups, void *data ATTRIBUTE_UNUSED) { + pool = pool; + groups = groups;
Hmm, I forgot to commit removal of this test statements :/ Consider this squashed in: diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 1f30b06..a7a7d0e 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -280,8 +280,6 @@ virStorageBackendDiskMakePoolGeometry(virStoragePoolObjPtr pool, char **const groups, void *data ATTRIBUTE_UNUSED) { - pool = pool; - groups = groups; 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 ||
+ 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; }
Peter

On 11/14/2013 09:20 AM, Peter Krempa wrote:
Also fix a typo in variable name holding the cylinders count of a disk pool (apparently unused).
If we're going to kill atoi(), let's also kill atof, atol, atoq, atoll. Oh, and maint.mk already has a rule that does this; we just have it disabled because we aren't clean yet (it also prohibits *scanf, because that also has undefined behavior on overflow; but we haven't scrubbed our sources to get rid of scanf usage).
+sc_prohibit_atoi: + @prohibit='\batoi *\(' \ + exclude='exempt from syntax-check' \
What are we exempting? You can probably drop this line.
+ halt='Use virStrToLong* instead of atoi' \
This at least is a more specific message than the generic check in maint.mk. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/14/13 17:28, Eric Blake wrote:
On 11/14/2013 09:20 AM, Peter Krempa wrote:
Also fix a typo in variable name holding the cylinders count of a disk pool (apparently unused).
If we're going to kill atoi(), let's also kill atof, atol, atoq, atoll.
Ok, these can be killed free of charge. None of them is used in libvirt code. I'll add them to the syntax-check rule.
Oh, and maint.mk already has a rule that does this; we just have it disabled because we aren't clean yet (it also prohibits *scanf, because that also has undefined behavior on overflow; but we haven't scrubbed our sources to get rid of scanf usage).
There's a ton of *scanf usage which will not be trivial to kill.
+sc_prohibit_atoi: + @prohibit='\batoi *\(' \ + exclude='exempt from syntax-check' \
What are we exempting? You can probably drop this line.
Right I now figured out what this is used for :).
+ halt='Use virStrToLong* instead of atoi' \
This at least is a more specific message than the generic check in maint.mk.
V2 comming soon. Peter
participants (2)
-
Eric Blake
-
Peter Krempa