[libvirt] [PATCH v2 8/8] util: Improve virStrncpy() implementation

[Andrea Bolognani]

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 at 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