[PATCH 0/5] Clean up various places using strcpy()
This series + https://listman.redhat.com/archives/libvir-list/2021-March/msg00081.html removes use of strcpy(). Peter Krempa (5): virIndexToDiskName: Make 'idx' unsigned and remove check virIndexToDiskName: Use g_string_prepend(_c) to improve readability remote_daemon_dispatch: Replace g_new + strcpy with g_strdup virHostCPUGetStatsLinux: Avoid 'strcpy' commandhelper: printCwd: Print result directly instead of copying it src/remote/remote_daemon_dispatch.c | 9 +++------ src/util/virhostcpu.c | 6 +++--- src/util/virutil.c | 29 +++++++---------------------- src/util/virutil.h | 2 +- src/vmx/vmx.c | 12 ------------ src/vz/vz_sdk.c | 3 --- tests/commandhelper.c | 7 ++++--- 7 files changed, 18 insertions(+), 50 deletions(-) -- 2.29.2
We can remove the check that 'idx' is negative by forcing callers to pass unsigned numbers, which they do already or have a check that 'idx' is positive. This in turn allows us to remove most return value NULL checks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virutil.c | 8 +------- src/util/virutil.h | 2 +- src/vmx/vmx.c | 12 ------------ src/vz/vz_sdk.c | 3 --- 4 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index a0cd0f1bcd..700ec02725 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -430,19 +430,13 @@ int virDiskNameToIndex(const char *name) return idx; } -char *virIndexToDiskName(int idx, const char *prefix) +char *virIndexToDiskName(unsigned int idx, const char *prefix) { char *name = NULL; size_t i; int ctr; int offset; - if (idx < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Disk index %d is negative"), idx); - return NULL; - } - for (i = 0, ctr = idx; ctr >= 0; ++i, ctr = ctr / 26 - 1) { } offset = strlen(prefix); diff --git a/src/util/virutil.h b/src/util/virutil.h index 46328727c1..854b494890 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -56,7 +56,7 @@ virFormatIntPretty(unsigned long long val, int virDiskNameParse(const char *name, int *disk, int *partition); int virDiskNameToIndex(const char* str); -char *virIndexToDiskName(int idx, const char *prefix); +char *virIndexToDiskName(unsigned int idx, const char *prefix); /* No-op workarounds for functionality missing in mingw. */ #ifndef WITH_GETUID diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 73bf7c4f3d..76d01a36de 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2217,9 +2217,6 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con (*def)->dst = virIndexToDiskName (controllerOrBus * 15 + (unit < 7 ? unit : unit - 1), "sd"); - - if ((*def)->dst == NULL) - goto cleanup; } else if (busType == VIR_DOMAIN_DISK_BUS_SATA) { if (controllerOrBus < 0 || controllerOrBus > 3) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2238,9 +2235,6 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con prefix = g_strdup_printf("sata%d:%d", controllerOrBus, unit); (*def)->dst = virIndexToDiskName(controllerOrBus * 30 + unit, "sd"); - - if ((*def)->dst == NULL) - goto cleanup; } else if (busType == VIR_DOMAIN_DISK_BUS_IDE) { if (controllerOrBus < 0 || controllerOrBus > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2258,9 +2252,6 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con prefix = g_strdup_printf("ide%d:%d", controllerOrBus, unit); (*def)->dst = virIndexToDiskName(controllerOrBus * 2 + unit, "hd"); - - if ((*def)->dst == NULL) - goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported bus type '%s' for device type '%s'"), @@ -2287,9 +2278,6 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con prefix = g_strdup_printf("floppy%d", unit); (*def)->dst = virIndexToDiskName(unit, "fd"); - - if ((*def)->dst == NULL) - goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported bus type '%s' for device type '%s'"), diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 6f712c7a31..d8548e5a3c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -626,9 +626,6 @@ prlsdkGetDiskId(PRL_HANDLE disk, int *bus, char **dst) return -1; } - if (NULL == *dst) - return -1; - return 0; } -- 2.29.2
Use a dynamic string helper so that we don't have to calculate the string lenghts and then iterate from the rear. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virutil.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 700ec02725..11e3e45615 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -432,24 +432,15 @@ int virDiskNameToIndex(const char *name) char *virIndexToDiskName(unsigned int idx, const char *prefix) { - char *name = NULL; - size_t i; - int ctr; - int offset; - - for (i = 0, ctr = idx; ctr >= 0; ++i, ctr = ctr / 26 - 1) { } - - offset = strlen(prefix); - - name = g_new0(char, offset + i + 1); + GString *str = g_string_new(NULL); + long long ctr; - strcpy(name, prefix); - name[offset + i] = '\0'; + for (ctr = idx; ctr >= 0; ctr = ctr / 26 - 1) + g_string_prepend_c(str, 'a' + (ctr % 26)); - for (i = i - 1, ctr = idx; ctr >= 0; --i, ctr = ctr / 26 - 1) - name[offset + i] = 'a' + (ctr % 26); + g_string_prepend(str, prefix); - return name; + return g_string_free(str, false); } #ifndef AI_CANONIDN -- 2.29.2
On 3/3/21 4:45 AM, Peter Krempa wrote:
Use a dynamic string helper so that we don't have to calculate the string lenghts and then iterate from the rear.
lengths
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virutil.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote/remote_daemon_dispatch.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index e9f2a0ce5b..9700dba450 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -2647,8 +2647,7 @@ remoteDispatchDomainGetSecurityLabel(virNetServerPtr server G_GNUC_UNUSED, goto cleanup; ret->label.label_len = strlen(seclabel->label) + 1; - ret->label.label_val = g_new0(char, ret->label.label_len); - strcpy(ret->label.label_val, seclabel->label); + ret->label.label_val = g_strdup(seclabel->label); ret->enforcing = seclabel->enforcing; rv = 0; @@ -2729,12 +2728,10 @@ remoteDispatchNodeGetSecurityModel(virNetServerPtr server G_GNUC_UNUSED, goto cleanup; ret->model.model_len = strlen(secmodel.model) + 1; - ret->model.model_val = g_new0(char, ret->model.model_len); - strcpy(ret->model.model_val, secmodel.model); + ret->model.model_val = g_strdup(secmodel.model); ret->doi.doi_len = strlen(secmodel.doi) + 1; - ret->doi.doi_val = g_new0(char, ret->doi.doi_len); - strcpy(ret->doi.doi_val, secmodel.doi); + ret->doi.doi_val = g_strdup(secmodel.doi); rv = 0; -- 2.29.2
Use an allocated buffer for 'cpu_header' so that g_strdup(_printf) can be used to fill it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virhostcpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 4f6c3390ce..2446948bc1 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -787,7 +787,7 @@ virHostCPUGetStatsLinux(FILE *procstat, char line[1024]; unsigned long long usr, ni, sys, idle, iowait; unsigned long long irq, softirq, steal, guest, guest_nice; - char cpu_header[4 + VIR_INT64_STR_BUFLEN]; + g_autofree char *cpu_header = NULL; if ((*nparams) == 0) { /* Current number of cpu stats supported by linux */ @@ -803,9 +803,9 @@ virHostCPUGetStatsLinux(FILE *procstat, } if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) { - strcpy(cpu_header, "cpu "); + cpu_header = g_strdup("cpu "); } else { - g_snprintf(cpu_header, sizeof(cpu_header), "cpu%d ", cpuNum); + cpu_header = g_strdup_printf("cpu%d ", cpuNum); } while (fgets(line, sizeof(line), procstat) != NULL) { -- 2.29.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/commandhelper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index ee06339392..9b56feb120 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -221,9 +221,10 @@ static int printCwd(FILE *log) if (!(cwd = getcwd(NULL, 0))) return -1; - if ((strlen(cwd) > strlen(".../commanddata")) && - (STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata"))) { - strcpy(cwd, ".../commanddata"); + if ((display = strstr(cwd, "/commanddata")) && + STREQ(display, "/commanddata")) { + fprintf(log, "CWD:.../commanddata\n"); + return 0; } display = cwd; -- 2.29.2
On 3/3/21 11:45 AM, Peter Krempa wrote:
This series + https://listman.redhat.com/archives/libvir-list/2021-March/msg00081.html
removes use of strcpy().
Peter Krempa (5): virIndexToDiskName: Make 'idx' unsigned and remove check virIndexToDiskName: Use g_string_prepend(_c) to improve readability remote_daemon_dispatch: Replace g_new + strcpy with g_strdup virHostCPUGetStatsLinux: Avoid 'strcpy' commandhelper: printCwd: Print result directly instead of copying it
src/remote/remote_daemon_dispatch.c | 9 +++------ src/util/virhostcpu.c | 6 +++--- src/util/virutil.c | 29 +++++++---------------------- src/util/virutil.h | 2 +- src/vmx/vmx.c | 12 ------------ src/vz/vz_sdk.c | 3 --- tests/commandhelper.c | 7 ++++--- 7 files changed, 18 insertions(+), 50 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Eric Blake -
Michal Privoznik -
Peter Krempa