[libvirt] [PATCH 0/3] A few more cleanups
Ján Tomko (3): Replace some uses STREQLEN with STRPREFIX Fix error detection in virStorageBackendISCSIGetHostNumber openvz: do not open-code STRSKIP src/openvz/openvz_conf.c | 3 +-- src/qemu/qemu_command.c | 2 +- src/storage/storage_backend_iscsi.c | 42 ++++++++++++++++++------------------- src/storage/storage_backend_scsi.c | 2 +- src/xen/xen_hypervisor.c | 4 ++-- 5 files changed, 26 insertions(+), 27 deletions(-) -- 2.7.3
Do not call it with a magic constant matching the length of the pattern. --- src/qemu/qemu_command.c | 2 +- src/storage/storage_backend_iscsi.c | 2 +- src/storage/storage_backend_scsi.c | 2 +- src/xen/xen_hypervisor.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08c66b8..0638a86 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3610,7 +3610,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (STREQLEN(def->os.machine, "s390-virtio", 10) && + if (STRPREFIX(def->os.machine, "s390-virtio") && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390) && def->memballoon) def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 98d1141..832cf65 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -106,7 +106,7 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, } while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { - if (STREQLEN(dirent->d_name, "target", strlen("target"))) { + if (STRPREFIX(dirent->d_name, "target")) { if (sscanf(dirent->d_name, "target%u:", host) != 1) { VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 558b3cf..99504f4 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -350,7 +350,7 @@ getBlockDevice(uint32_t host, goto cleanup; while ((direrr = virDirRead(lun_dir, &lun_dirent, lun_path)) > 0) { - if (STREQLEN(lun_dirent->d_name, "block", 5)) { + if (STRPREFIX(lun_dirent->d_name, "block")) { if (strlen(lun_dirent->d_name) == 5) { if (getNewStyleBlockDevice(lun_path, lun_dirent->d_name, diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index fc9e1c6..79b25b3 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2255,7 +2255,7 @@ get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int *longmode) *pae = 0; *hvm = ""; - if (STREQLEN((const char *)®s.r_ebx, "AuthcAMDenti", 12)) { + if (STRPREFIX((const char *)®s.r_ebx, "AuthcAMDenti")) { if (pread(fd, ®s, sizeof(regs), 0x80000001) == sizeof(regs)) { /* Read secure virtual machine bit (bit 2 of ECX feature ID) */ if ((regs.r_ecx >> 2) & 1) @@ -2263,7 +2263,7 @@ get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int *longmode) if ((regs.r_edx >> 6) & 1) *pae = 1; } - } else if (STREQLEN((const char *)®s.r_ebx, "GenuntelineI", 12)) { + } else if (STRPREFIX((const char *)®s.r_ebx, "GenuntelineI")) { if (pread(fd, ®s, sizeof(regs), 0x00000001) == sizeof(regs)) { /* Read VMXE feature bit (bit 5 of ECX feature ID) */ if ((regs.r_ecx >> 5) & 1) -- 2.7.3
On Fri, 2016-06-24 at 15:00 +0200, Ján Tomko wrote:
Do not call it with a magic constant matching the length of the pattern. --- src/qemu/qemu_command.c | 2 +- src/storage/storage_backend_iscsi.c | 2 +- src/storage/storage_backend_scsi.c | 2 +- src/xen/xen_hypervisor.c | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08c66b8..0638a86 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3610,7 +3610,7 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, { virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (STREQLEN(def->os.machine, "s390-virtio", 10) && + if (STRPREFIX(def->os.machine, "s390-virtio") &&
The original code only checked the first 10 bytes of def->os.machine, though. So it would have matched "s390-virtie", but the new version doesn't! :P -- Andrea Bolognani / Red Hat / Virtualization
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value. Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 40 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 832cf65..bb33da2 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -91,7 +91,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -100,27 +100,32 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, virFileWaitForDevices(); - if (virDirOpen(&sysdir, sysfs_path) < 0) { - retval = -1; - goto out; - } + if (virDirOpen(&sysdir, sysfs_path) < 0) + goto cleanup; while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STRPREFIX(dirent->d_name, "target")) { - if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1; + if (direrr == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to get host number for iSCSI session " + "with path '%s'"), sysfs_path); + goto cleanup; + } + + cleanup: VIR_DIR_CLOSE(sysdir); - out: - return retval; + return ret; } static int @@ -135,13 +140,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, "/sys/class/iscsi_session/session%s/device", session) < 0) goto cleanup; - if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) { - virReportSystemError(errno, - _("Failed to get host number for iSCSI session " - "with path '%s'"), - sysfs_path); + if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) goto cleanup; - } if (virStorageBackendSCSIFindLUs(pool, host) < 0) goto cleanup; -- 2.7.3
Remove one more use of STREQLEN with strlen as its argument. --- src/openvz/openvz_conf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index ff32191..1403350 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -737,10 +737,9 @@ openvzReadConfigParam(const char *conf_file, const char *param, char **value) break; } - if (! STREQLEN(line, param, strlen(param))) + if (!(sf = STRSKIP(line, param))) continue; - sf = line + strlen(param); if (*sf++ != '=') continue; -- 2.7.3
On 24/06/16 15:00, Ján Tomko wrote:
Ján Tomko (3): Replace some uses STREQLEN with STRPREFIX Fix error detection in virStorageBackendISCSIGetHostNumber openvz: do not open-code STRSKIP
src/openvz/openvz_conf.c | 3 +-- src/qemu/qemu_command.c | 2 +- src/storage/storage_backend_iscsi.c | 42 ++++++++++++++++++------------------- src/storage/storage_backend_scsi.c | 2 +- src/xen/xen_hypervisor.c | 4 ++-- 5 files changed, 26 insertions(+), 27 deletions(-)
ACK series. Erik
participants (3)
-
Andrea Bolognani -
Erik Skultety -
Ján Tomko