9:44 a.m.
We finally get rid of the strncpy()-like semantics
and implement our own, more sensible ones instead.
As a bonus, this also fixes compilation on MinGW.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
docs/hacking.html.in | 29 ++++++++++-----------
src/util/virstring.c | 62 ++++++++++++++++++++++++++++++--------------
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 6c1a5121a4..f99d143b7b 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1121,22 +1121,22 @@
<p>
Do not use the strncpy function. According to the man page, it
does <b>not</b> guarantee a NULL-terminated buffer, which makes
- it extremely dangerous to use. Instead, use one of the
- functionally equivalent functions:
+ it extremely dangerous to use. Instead, use one of the replacement
+ functions provided by libvirt:
</p>
<pre>
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
</pre>
<p>
- The first three arguments have the same meaning as for strncpy;
- namely the destination, source, and number of bytes to copy,
- respectively. The last argument is the number of bytes
- available in the destination string; if a copy of the source
- string (including a \0) will not fit into the destination, no
- bytes are copied and the routine returns <0. Otherwise, n
- bytes from the source are copied into the destination and a
- trailing \0 is appended.
+ The first two arguments have the same meaning as for strncpy,
+ namely the destination and source of the copy operation. Unlike
+ strncpy, the function will always copy exactly the number of bytes
+ requested and make sure the destination is NULL-terminated, as the
+ source is required to be; sanity checks are performed to ensure the
+ size of the destination, as specified by the last argument, is
+ sufficient for the operation to succeed. On success, 0 is returned;
+ on failure, a value <0 is returned instead.
</p>
<pre>
@@ -1144,10 +1144,8 @@
</pre>
<p>
Use this variant if you know you want to copy the entire src
- string into dest. Note that this is a macro, so arguments could
- be evaluated more than once. This is equivalent to
- virStrncpy(dest, src, strlen(src), destbytes)
- </p>
+ string into dest.
+ </p>
<pre>
virStrcpyStatic(char *dest, const char *src)
@@ -1157,8 +1155,7 @@
string into dest <b>and</b> you know that your destination string is
a static string (i.e. that sizeof(dest) returns something
meaningful). Note that this is a macro, so arguments could be
- evaluated more than once. This is equivalent to
- virStrncpy(dest, src, strlen(src), sizeof(dest)).
+ evaluated more than once.
</p>
<pre>
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 3e2f85465f..93fda69d7f 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -769,44 +769,66 @@ virAsprintfInternal(bool report,
}
/**
- * virStrncpy
+ * virStrncpy:
*
- * A safe version of strncpy. The last parameter is the number of bytes
- * available in the destination string, *not* the number of bytes you want
- * to copy. If the destination is not large enough to hold all n of the
- * src string bytes plus a \0, <0 is returned and no data is copied.
- * If the destination is large enough to hold the n bytes plus \0, then the
- * string is copied and 0 is returned.
+ * @dest: destination buffer
+ * @src: source buffer
+ * @n: number of bytes to copy
+ * @destbytes: number of bytes the destination can accomodate
+ *
+ * Copies the first @n bytes of @src to @dest.
+ *
+ * @src must be NULL-terminated; if successful, @dest is guaranteed to
+ * be NULL-terminated as well.
+ *
+ * @n must be a reasonable value, that is, it must not exceed either
+ * the length of @src or the size of @dest. For the latter constraint,
+ * the fact that @dest needs to accomodate a NULL byte in addition to
+ * the bytes copied from @src must be taken into account.
+ *
+ * If you want to copy *all* of @src to @dest, use virStrcpy() or
+ * virStrcpyStatic() instead.
+ *
+ * Returns: 0 on success, <0 on failure.
*/
int
virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
{
- if (n > (destbytes - 1))
+ size_t src_len = strlen(src);
+
+ /* As a special case, -1 means "copy the entire string".
+ *
+ * This is to avoid calling strlen() twice, once in the virStrcpy()
+ * wrapper and once here for bound checking purposes. */
+ if (n == -1)
+ n = src_len;
+
+ if (n <= 0 || n > src_len || n > (destbytes - 1))
return -1;
- strncpy(dest, src, n);
- /* strncpy NULL terminates iff the last character is \0. Therefore
- * force the last byte to be \0
- */
+ memcpy(dest, src, n);
dest[n] = '\0';
return 0;
}
/**
- * virStrcpy
+ * virStrcpy:
+ *
+ * @dest: destination buffer
+ * @src: source buffer
+ * @destbytes: number of bytes the destination can accomodate
+ *
+ * Copies @src to @dest.
+ *
+ * See virStrncpy() for more information.
*
- * A safe version of strcpy. The last parameter is the number of bytes
- * available in the destination string, *not* the number of bytes you want
- * to copy. If the destination is not large enough to hold all n of the
- * src string bytes plus a \0, <0 is returned and no data is copied.
- * If the destination is large enough to hold the source plus \0, then the
- * string is copied and 0 is returned.
+ * Returns: 0 on success, <0 on failure.
*/
int
virStrcpy(char *dest, const char *src, size_t destbytes)
{
- return virStrncpy(dest, src, strlen(src), destbytes);
+ return virStrncpy(dest, src, -1, destbytes);
}
/**
--
2.17.1