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

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Thu, Feb 23, 2017 at 04:26:57PM +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> 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
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list