Re: [libvirt] [PATCH 2/5] util: use VIR_STRCAT instead of strcat

On Thu, Feb 23, 2017 at 04:26:57PM +0100, Pavel Hrdina wrote: >Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >--- > cfg.mk | 14 ++++++++------ > src/storage/storage_backend_logical.c | 6 +++--- > src/test/test_driver.c | 2 +- > src/util/vircgroup.c | 4 +--- > src/xen/xend_internal.c | 2 +- > 5 files changed, 14 insertions(+), 14 deletions(-) > >diff --git a/cfg.mk b/cfg.mk >index 22c655eac6..6646509206 100644 >--- a/cfg.mk >+++ b/cfg.mk >@@ -390,6 +390,11 @@ sc_prohibit_strdup: > halt='use VIR_STRDUP, not strdup' \ > $(_sc_search_regexp) > >+sc_prohibit_strcat: >+ @prohibit='\<strn?cat\> *\(' \ >+ halt='use VIR_STRCAT, not strcat' \ >+ $(_sc_search_regexp) >+ > # Prefer virSetUIDGID. > sc_prohibit_setuid: > @prohibit='\<set(re)?[ug]id\> *\(' \ >@@ -994,12 +999,6 @@ sc_prohibit_not_streq: > halt='Use STRNEQ instead of !STREQ and STREQ instead of !STRNEQ' \ > $(_sc_search_regexp) > >-sc_prohibit_verbose_strcat: >- @prohibit='strncat\([^,]*,\s+([^,]*),\s+strlen\(\1\)\)' \ >- in_vc_files='\.[ch]$$' \ >- halt='Use strcat(a, b) instead of strncat(a, b, strlen(b))' \ >- $(_sc_search_regexp) >- > # Ensure that each .c file containing a "main" function also > # calls virGettextInitialize > sc_gettext_init: >@@ -1134,6 +1133,9 @@ exclude_file_name_regexp--sc_prohibit_asprintf = \ > exclude_file_name_regexp--sc_prohibit_strdup = \ > ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) > >+exclude_file_name_regexp--sc_prohibit_strcat = \ >+ ^(docs/|src/util/virstring\.c|tests/virstringtest\.c)$$ why virstringtest.c?

>+ > exclude_file_name_regexp--sc_prohibit_close = \ > (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir.+mock\.c)$$) > >diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c >index 756c62e908..5e006a980a 100644 >--- a/src/storage/storage_backend_logical.c >+++ b/src/storage/storage_backend_logical.c >@@ -201,11 +201,11 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, > /* Allocate space for 'nextents' regex_unit strings plus a comma for each */ > if (VIR_ALLOC_N(regex, nextents * (strlen(regex_unit) + 1) + 1) < 0) > goto cleanup; >- strcat(regex, regex_unit); >+ VIR_STRCAT_INPLACE(regex, regex_unit); > for (i = 1; i < nextents; i++) { > /* "," is the separator of "devices" field */ >- strcat(regex, ","); >- strcat(regex, regex_unit); >+ VIR_STRCAT_INPLACE(regex, ","); >+ VIR_STRCAT_INPLACE(regex, regex_unit); > } > > if (VIR_ALLOC(reg) < 0) >diff --git a/src/test/test_driver.c b/src/test/test_driver.c >index 5fef3f10b9..be887ec1bb 100644 >--- a/src/test/test_driver.c >+++ b/src/test/test_driver.c >@@ -699,7 +699,7 @@ static char *testBuildFilename(const char *relativeTo, > VIR_FREE(absFile); > return NULL; > } >- strcat(absFile, filename); >+ ignore_value(VIR_STRCAT_INPLACE(absFile, filename)); > return absFile; > } else { > ignore_value(VIR_STRDUP(ret, filename)); >diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c >index 5aa1db5b14..e8210ca6eb 100644 >--- a/src/util/vircgroup.c >+++ b/src/util/vircgroup.c >@@ -1299,10 +1299,8 @@ virCgroupSetPartitionSuffix(const char *path, char **res) > */ > if (STRNEQ(tokens[i], "") && > !strchr(tokens[i], '.')) { >- if (VIR_REALLOC_N(tokens[i], >- strlen(tokens[i]) + strlen(".partition") + 1) < 0) >+ if (VIR_STRCAT(tokens[i], ".partition") < 0) > goto cleanup; >- strcat(tokens[i], ".partition"); Not counting the rest of your patches, just for now, is this the only place the VIR_STRCAT adds value? This makes me even more cautious about the patches.

> } > > if (virCgroupPartitionEscape(&(tokens[i])) < 0) >diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c >index 605c3cdccf..1f9d4c6959 100644 >--- a/src/xen/xend_internal.c >+++ b/src/xen/xend_internal.c >@@ -1824,7 +1824,7 @@ xenDaemonDomainPinVcpu(virConnectPtr conn, > for (i = 0; i < maplen; i++) for (j = 0; j < 8; j++) > if (cpumap[i] & (1 << j)) { > snprintf(buf, sizeof(buf), "%zu,", (8 * i) + j); >- strcat(mapstr, buf); >+ VIR_STRCAT_INPLACE(mapstr, buf); > } > mapstr[strlen(mapstr) - 1] = 0; > >-- >2.11.1 > >-- >libvir-list mailing list &gt;libvir-list(a)redhat.com >https://www.redhat.com/mailman/listinfo/libvir-list