1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/5] virstring and virbuffer improvements and bug fix
  4. View patch

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

Pavel Hrdina posted 5 patches 8 years, 11 months ago
[libvirt] [PATCH 2/5] util: use VIR_STRCAT instead of strcat
Posted by Pavel Hrdina 8 years, 11 months ago 
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 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)$$
+
 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");
         }
 
         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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: use VIR_STRCAT instead of strcat
Posted by Martin Kletzander 8 years, 11 months ago 
On Thu, Feb 23, 2017 at 04:26:57PM +0100, Pavel Hrdina wrote:
>Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>---
> 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@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: use VIR_STRCAT instead of strcat
Posted by Pavel Hrdina 8 years, 11 months ago 
On Thu, Feb 23, 2017 at 05:15:37PM +0100, Martin Kletzander wrote:
> On Thu, Feb 23, 2017 at 04:26:57PM +0100, Pavel Hrdina wrote:
> >Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> >---
> > 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?

I'll remove it, nice catch.

> >+
> > 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.

Yes, the VIR_STRCAT was introduced solely for the following patch and since
there was code using strcat I though that it would be nice to switch to
VIR_STRCAT.

It definitely doesn't hurt if we have this helper and who knows, someone may
use it some day :).

Pavel

> >         }
> >
> >         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@redhat.com
> >https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] util: use VIR_STRCAT instead of strcat
Posted by Daniel P. Berrange 8 years, 11 months ago 
On Thu, Feb 23, 2017 at 05:15:37PM +0100, Martin Kletzander wrote:
> On Thu, Feb 23, 2017 at 04:26:57PM +0100, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> > 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.

That code could be rewritten with virAsprintf for clarity eg

           char *part;
	   if (virAsprintf(&part, "%s.partition", tokens[i]) < 0)
	        goto cleanup;
           VIR_FREE(tokens[i]);
	   tokens[i] = part;

No, it doesn't make it shorter, but I think it makes it clearer as
to what we're actually doing here.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

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

Patchew 2.1-devel-8c64711

© 2016 - 2026 Red Hat, Inc.