[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/5] util: virstring: introduce virStrcat and VIR_STRCAT



On Thu, Feb 23, 2017 at 04:26:56PM +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina redhat com>
---
cfg.mk                   |  2 +-
src/libvirt_private.syms |  2 ++
src/util/virstring.c     | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h     | 27 +++++++++++++++++++
tests/virstringtest.c    | 49 +++++++++++++++++++++++++++++++++
5 files changed, 149 insertions(+), 1 deletion(-)

diff --git a/cfg.mk b/cfg.mk
index aaba61f1dc..22c655eac6 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1147,7 +1147,7 @@ exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/virutil\.c$$

exclude_file_name_regexp--sc_prohibit_internal_functions = \
-  ^src/(util/(viralloc|virutil|virfile)\.[hc]|esx/esx_vi\.c)$$
+  ^src/(util/(viralloc|virutil|virfile|virstring)\.[hc]|esx/esx_vi\.c)$$

exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
  ^src/rpc/gendispatch\.pl$$
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 07a35333b1..e9c4d73779 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2502,6 +2502,8 @@ virAsprintfInternal;
virSkipSpaces;
virSkipSpacesAndBackslash;
virSkipSpacesBackwards;
+virStrcat;
+virStrcatInplace;
virStrcpy;
virStrdup;
virStringBufferIsPrintable;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 69abc267bf..bc15ce7e9e 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -837,6 +837,76 @@ virStrndup(char **dest,
}


+/**
+ * virStrcat
+ * @dest: where to store concatenated string
+ * @src: the source string to append to @dest
+ * @inPlace: false if we should expand the allocated memory before moving,
+ *           true if we should assume someone else has already done that.

This is here probably from some work in progress version.

+ * @report: whether to report OOM error, if there is one
+ * @domcode: error domain code
+ * @filename: caller's filename
+ * @funcname: caller's funcname
+ * @linenr: caller's line number
+ *
+ * Wrapper over strcat, which reports OOM error if told so,
+ * in which case callers wants to pass @domcode, @filename,
+ * @funcname and @linenr which should represent location in
+ * caller's body where virStrcat is called from. Consider
+ * using VIR_STRCAT which sets these automatically.
+ *
+ * Returns: 0 for NULL src, 1 on successful concatenate, -1 otherwise.
+ */
+int
+virStrcat(char **dest,
+          const char *src,
+          bool report,
+          int domcode,
+          const char *filename,
+          const char *funcname,
+          size_t linenr)
+{
+    size_t dest_len = 0;
+    size_t src_len = 0;
+
+    if (!src)
+        return 0;
+
+    if (*dest)
+        dest_len = strlen(*dest);
+    src_len = strlen(src);
+
+    if (virReallocN(dest, sizeof(*dest), dest_len + src_len + 1,
+                    report, domcode, filename, funcname, linenr) < 0)
+        return -1;
+
+    strcat(*dest, src);
+
+    return 1;
+}
+
+
+/**
+ * virStrcat
+ * @dest: where to store concatenated string
+ * @src: the source string to append to @dest
+ *
+ * Wrapper over strcat, which properly handles if @src is NULL.
+ *
+ * Returns: 0 for NULL src, 1 on successful concatenate.
+ */

Really?  This whole wrapper just for checking NULL?  So instead of:

if (x) strcat (y, x)

I should do:

VIR_STRCAT_INPLACE(y, x) now?  It's not even saving any characters.

Plus, is there *really* any occurrence of strcat that might be called
with NULL?  I would say that such code has more logic problems in that
case.

The reallocating strcat makes sense, but the name is *really* confusing
for people who are used to what strcat/strncat does.  So while I see how
that might be useful, we also have a buffer for more interesting dynamic
string modifications and I'm not sure this is something that will help a
lot.

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]