[libvirt] [PATCHv3] conf: don't modify cpu set string during parsing

None of the callers cared if str was updated to point to the next byte after the parsed cpuset; simplifying this results in quite a few code simplifications. Additionally, virCPUDefParseXML was strdup()'ing a malloc()'d string; avoiding a memory copy resulted in less code. * src/conf/domain_conf.h (virDomainCpuSetParse): Alter signature. * src/conf/domain_conf.c (virDomainCpuSetParse): Don't modify str. (virDomainVcpuPinDefParseXML, virDomainDefParseXML): Adjust callers. * src/conf/cpu_conf.c (virCPUDefParseXML): Likewise. * src/xen/xend_internal.c (sexpr_to_xend_topology): Likewise. * src/xen/xm_internal.c (xenXMDomainPinVcpu): Likewise. * src/xenxs/xen_sxpr.c (xenParseSxpr): Likewise. * src/xenxs/xen_xm.c (xenParseXM): Likewise. --- I'm only proposing this for patch 1/11; 2-11 of v2 might have to change slightly to rebase on top of this. v3: change parameter type and simplify callers, now that we know no one cares about adjusting the parsed string pointer. src/conf/cpu_conf.c | 26 +++++++------------------- src/conf/domain_conf.c | 17 ++++++----------- src/conf/domain_conf.h | 2 +- src/xen/xend_internal.c | 2 +- src/xen/xm_internal.c | 3 +-- src/xenxs/xen_sxpr.c | 3 +-- src/xenxs/xen_xm.c | 2 +- 7 files changed, 18 insertions(+), 37 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 7c5bf69..2882389 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -311,42 +311,32 @@ virCPUDefParseXML(const xmlNodePtr node, def->ncells = n; for (i = 0 ; i < n ; i++) { - char *cpus, *cpus_parse, *memory; + char *cpus, *memory; int cpumasklen = VIR_DOMAIN_CPUMASK_LEN; int ret, ncpus = 0; def->cells[i].cellid = i; - cpus = cpus_parse = virXMLPropString(nodes[i], "cpus"); + cpus = virXMLPropString(nodes[i], "cpus"); if (!cpus) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing 'cpus' attribute in NUMA cell")); goto error; } + def->cells[i].cpustr = cpus; - def->cells[i].cpustr = strdup(cpus); - if (!def->cells[i].cpustr) { - VIR_FREE(cpus); + if (VIR_ALLOC_N(def->cells[i].cpumask, cpumasklen) < 0) goto no_memory; - } - if (VIR_ALLOC_N(def->cells[i].cpumask, cpumasklen) < 0) { - VIR_FREE(cpus); - goto no_memory; - } - - ncpus = virDomainCpuSetParse((const char **)&cpus_parse, - 0, def->cells[i].cpumask, cpumasklen); - if (ncpus <= 0) { - VIR_FREE(cpus); + ncpus = virDomainCpuSetParse(cpus, 0, def->cells[i].cpumask, + cpumasklen); + if (ncpus <= 0) goto error; - } def->cells_cpus += ncpus; memory = virXMLPropString(nodes[i], "memory"); if (!memory) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing 'memory' attribute in NUMA cell")); - VIR_FREE(cpus); goto error; } @@ -354,11 +344,9 @@ virCPUDefParseXML(const xmlNodePtr node, if (ret == -1) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid 'memory' attribute in NUMA cell")); - VIR_FREE(cpus); VIR_FREE(memory); goto error; } - VIR_FREE(cpus); VIR_FREE(memory); } } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 452c8b2..0620bb3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6560,8 +6560,7 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, virReportOOMError(); goto error; } - if (virDomainCpuSetParse((const char **)&set, - 0, def->cpumask, + if (virDomainCpuSetParse(set, 0, def->cpumask, cpumasklen) < 0) goto error; VIR_FREE(tmp); @@ -6774,8 +6773,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (VIR_ALLOC_N(def->cpumask, def->cpumasklen) < 0) { goto no_memory; } - if (virDomainCpuSetParse((const char **)&set, - 0, def->cpumask, + if (virDomainCpuSetParse(set, 0, def->cpumask, def->cpumasklen) < 0) goto error; VIR_FREE(tmp); @@ -6845,8 +6843,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } /* "nodeset" leads same syntax with "cpuset". */ - if (virDomainCpuSetParse((const char **)&set, - 0, def->numatune.memory.nodemask, + if (virDomainCpuSetParse(set, 0, def->numatune.memory.nodemask, nodemasklen) < 0) goto error; VIR_FREE(tmp); @@ -9079,7 +9076,7 @@ virDomainCpuSetFormat(char *cpuset, int maxcpu) /** * virDomainCpuSetParse: * @conn: connection - * @str: pointer to a CPU set string pointer + * @str: a CPU set string pointer * @sep: potential character used to mark the end of string if not 0 * @cpuset: pointer to a char array for the CPU set * @maxcpu: number of elements available in @cpuset @@ -9090,10 +9087,9 @@ virDomainCpuSetFormat(char *cpuset, int maxcpu) * * Returns the number of CPU found in that set, or -1 in case of error. * @cpuset is modified accordingly to the value parsed. - * @str is updated to the end of the part parsed */ int -virDomainCpuSetParse(const char **str, char sep, +virDomainCpuSetParse(const char *str, char sep, char *cpuset, int maxcpu) { const char *cur; @@ -9105,7 +9101,7 @@ virDomainCpuSetParse(const char **str, char sep, (maxcpu > 100000)) return (-1); - cur = *str; + cur = str; virSkipSpaces(&cur); if (*cur == 0) goto parse_error; @@ -9170,7 +9166,6 @@ virDomainCpuSetParse(const char **str, char sep, } else goto parse_error; } - *str = cur; return (ret); parse_error: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4e86d30..4a19dd2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1745,7 +1745,7 @@ int virDomainDefFormatInternal(virDomainDefPtr def, unsigned int flags, virBufferPtr buf); -int virDomainCpuSetParse(const char **str, +int virDomainCpuSetParse(const char *str, char sep, char *cpuset, int maxcpu); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 80a2472..2318565 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1153,7 +1153,7 @@ sexpr_to_xend_topology(const struct sexpr *root, for (cpu = 0; cpu < numCpus; cpu++) cpuset[cpu] = 0; } else { - nb_cpus = virDomainCpuSetParse(&cur, 'n', cpuset, numCpus); + nb_cpus = virDomainCpuSetParse(cur, 'n', cpuset, numCpus); if (nb_cpus < 0) goto error; } diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 24311a7..a250b6f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -893,8 +893,7 @@ int xenXMDomainPinVcpu(virDomainPtr domain, virReportOOMError(); goto cleanup; } - if (virDomainCpuSetParse((const char **)&mapstr, 0, - cpuset, maxcpu) < 0) + if (virDomainCpuSetParse(mapstr, 0, cpuset, maxcpu) < 0) goto cleanup; VIR_FREE(entry->def->cpumask); diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index e0bc043..124c369 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1140,8 +1140,7 @@ xenParseSxpr(const struct sexpr *root, goto error; } - if (virDomainCpuSetParse(&cpus, - 0, def->cpumask, + if (virDomainCpuSetParse(cpus, 0, def->cpumask, def->cpumasklen) < 0) { XENXS_ERROR(VIR_ERR_INTERNAL_ERROR, _("invalid CPU mask %s"), cpus); diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index a2ef8c8..b415ce8 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -338,7 +338,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (VIR_ALLOC_N(def->cpumask, def->cpumasklen) < 0) goto no_memory; - if (virDomainCpuSetParse(&str, 0, + if (virDomainCpuSetParse(str, 0, def->cpumask, def->cpumasklen) < 0) goto cleanup; } -- 1.7.7.1