string: make VIR_STRDUP easier to use

While reviewing proposed VIR_STRDUP conversions, I've already noticed
several places that do:

if (str && VIR_STRDUP(dest, str) < 0)

which can be simplified by allowing str to be NULL (something that
strdup() doesn't allow).  Meanwhile, code that wants to ensure a
non-NULL dest regardless of the source can check for <= 0.

Also, make it part of the VIR_STRDUP contract that macro arguments
are evaluated exactly once.

* src/util/virstring.h (VIR_STRDUP, VIR_STRDUP_QUIET, VIR_STRNDUP)
(VIR_STRNDUP_QUIET): Improve contract.
* src/util/virstring.c (virStrdup, virStrndup): Change return
conventions.
* docs/hacking.html.in: Document this.
* HACKING: Regenerate.

Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2013-05-06 20:42:44 -06:00
parent ddcfc5492a
commit 6b74a9f5d9
4 changed files with 36 additions and 21 deletions

14
HACKING
View File

@ -421,7 +421,7 @@ Preprocessor
Macros defined with an ALL_CAPS name should generally be assumed to be unsafe
with regards to arguments with side-effects (that is, MAX(a++, b--) might
increment a or decrement b too many or too few times). Exceptions to this rule
are explicitly documented for macros in viralloc.h.
are explicitly documented for macros in viralloc.h and virstring.h.
For variadic macros, stick with C99 syntax:
@ -728,12 +728,12 @@ virStrncpy(dest, src, strlen(src), sizeof(dest)).
VIR_STRNDUP(char *dst, const char *src, size_t n);
You should avoid using strdup or strndup directly as they do not report
out-of-memory error. Use VIR_STRDUP or VIR_STRNDUP macros instead. Note, that
these two behave similar to VIR_ALLOC: on success zero is returned, otherwise
the result is -1 and dst is guaranteed to be NULL. In very specific cases,
when you don't want to report the out-of-memory error, you can use
VIR_STRDUP_QUIET or VIR_STRNDUP_QUIET, but such usage is very rare and usually
considered a flaw.
out-of-memory error, and do not allow a NULL source. Use VIR_STRDUP or
VIR_STRNDUP macros instead, which return 0 for NULL source, 1 for successful
copy, and -1 for allocation failure with the error already reported. In very
specific cases, when you don't want to report the out-of-memory error, you can
use VIR_STRDUP_QUIET or VIR_STRNDUP_QUIET, but such usage is very rare and
usually considered a flaw.
Variable length string buffer

View File

@ -523,7 +523,7 @@
assumed to be unsafe with regards to arguments with side-effects
(that is, MAX(a++, b--) might increment a or decrement b too
many or too few times). Exceptions to this rule are explicitly
documented for macros in viralloc.h.
documented for macros in viralloc.h and virstring.h.
</p>
<p>
@ -868,9 +868,10 @@
</pre>
<p>
You should avoid using strdup or strndup directly as they do not report
out-of-memory error. Use VIR_STRDUP or VIR_STRNDUP macros instead. Note,
that these two behave similar to VIR_ALLOC: on success zero is returned,
otherwise the result is -1 and dst is guaranteed to be NULL. In very
out-of-memory error, and do not allow a NULL source. Use
VIR_STRDUP or VIR_STRNDUP macros instead, which return 0 for
NULL source, 1 for successful copy, and -1 for allocation
failure with the error already reported. In very
specific cases, when you don't want to report the out-of-memory error, you
can use VIR_STRDUP_QUIET or VIR_STRNDUP_QUIET, but such usage is very rare
and usually considered a flaw.

View File

@ -532,7 +532,7 @@ virArgvToString(const char *const *argv)
* caller's body where virStrdup is called from. Consider
* using VIR_STRDUP which sets these automatically.
*
* Returns: 0 on success, -1 otherwise.
* Returns: 0 for NULL src, 1 on successful copy, -1 otherwise.
*/
int
virStrdup(char **dest,
@ -543,13 +543,15 @@ virStrdup(char **dest,
const char *funcname,
size_t linenr)
{
if (!src)
return 0;
if (!(*dest = strdup(src))) {
if (report)
virReportOOMErrorFull(domcode, filename, funcname, linenr);
return -1;
}
return 0;
return 1;
}
/**
@ -569,7 +571,7 @@ virStrdup(char **dest,
* caller's body where virStrndup is called from. Consider
* using VIR_STRNDUP which sets these automatically.
*
* Returns: 0 on success, -1 otherwise.
* Returns: 0 for NULL src, 1 on successful copy, -1 otherwise.
*/
int
virStrndup(char **dest,
@ -581,11 +583,13 @@ virStrndup(char **dest,
const char *funcname,
size_t linenr)
{
if (!src)
return 0;
if (!(*dest = strndup(src, n))) {
if (report)
virReportOOMErrorFull(domcode, filename, funcname, linenr);
return -1;
}
return 0;
return 1;
}

View File

@ -92,11 +92,11 @@ char *virStrcpy(char *dest, const char *src, size_t destbytes)
/* Don't call these directly - use the macros below */
int virStrdup(char **dest, const char *src, bool report, int domcode,
const char *filename, const char *funcname, size_t linenr)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
const char *filename, const char *funcname, size_t linenr)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
/**
* VIR_STRDUP:
@ -105,7 +105,10 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
*
* Duplicate @src string and store it into @dst.
*
* Returns -1 on failure (with OOM error reported), 0 on success
* This macro is safe to use on arguments with side effects.
*
* Returns -1 on failure (with OOM error reported), 0 if @src was NULL,
* 1 if @src was copied
*/
# define VIR_STRDUP(dst, src) virStrdup(&(dst), src, true, VIR_FROM_THIS, \
__FILE__, __FUNCTION__, __LINE__)
@ -117,7 +120,9 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
*
* Duplicate @src string and store it into @dst.
*
* Returns -1 on failure, 0 on success
* This macro is safe to use on arguments with side effects.
*
* Returns -1 on failure, 0 if @src was NULL, 1 if @src was copied
*/
# define VIR_STRDUP_QUIET(dst, src) virStrdup(&(dst), src, false, 0, NULL, NULL, 0)
@ -130,7 +135,10 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
* Duplicate @src string and store it into @dst. If @src is longer than @n,
* only @n bytes are copied and terminating null byte '\0' is added.
*
* Returns -1 on failure (with OOM error reported), 0 on success
* This macro is safe to use on arguments with side effects.
*
* Returns -1 on failure (with OOM error reported), 0 if @src was NULL,
* 1 if @src was copied
*/
# define VIR_STRNDUP(dst, src, n) virStrndup(&(dst), src, n, true, \
VIR_FROM_THIS, __FILE__, \
@ -145,7 +153,9 @@ int virStrndup(char **dest, const char *src, size_t n, bool report, int domcode,
* Duplicate @src string and store it into @dst. If @src is longer than @n,
* only @n bytes are copied and terminating null byte '\0' is added.
*
* Returns -1 on failure, 0 on success
* This macro is safe to use on arguments with side effects.
*
* Returns -1 on failure, 0 if @src was NULL, 1 if @src was copied
*/
# define VIR_STRNDUP_QUIET(dst, src, n) virStrndup(&(dst), src, n, false, \
0, NULL, NULL, 0)