10:15 a.m.
On Thu, Feb 23, 2017 at 05:06:53PM +0100, Martin Kletzander wrote:
On Thu, Feb 23, 2017 at 04:26:56PM +0100, Pavel Hrdina wrote:
>Signed-off-by: Pavel Hrdina <phrdina(a)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 reason is to forbid using strcat directly and force to use the wrappers.
I had two options in my mind, one that will use the virStrcatInplace function
with return values 0 and 1 to determine whether something actually happened or
only the "if (x) strcat (y, x)" as a macro without any return value.
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
In that case it would be nice to provide some alternative :).
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.
Yes we have buffer, but this macro will be used in the buffer code itself
and I guess it would be strange to use a buffer inside a buffer function.
Pavel