Moreover, the original virVasprintf and virAsprintf functions will report OOM error. The new behaviour is not enabled for now as the rest of code needs to be adapted so we don't report an error twice. --- HACKING | 4 ++++ docs/hacking.html.in | 6 ++++++ src/libvirt_private.syms | 2 ++ src/util/viraudit.c | 4 +--- src/util/vircommand.c | 4 ++-- src/util/virerror.c | 2 +- src/util/virlog.c | 2 +- src/util/virstring.c | 44 ++++++++++++++++++++++++++++++++++++++++---- src/util/virstring.h | 4 ++++ tools/virsh.c | 6 +++--- 10 files changed, 64 insertions(+), 14 deletions(-) diff --git a/HACKING b/HACKING index 842f476..0f681ce 100644 --- a/HACKING +++ b/HACKING @@ -818,6 +818,10 @@ virAsprintf, in util.h: This makes it so gcc's -Wformat and -Wformat-security options can do their jobs and cross-check format strings with the number and types of arguments. +Moreover, this function automatically reports out-of-memory error if there's +any. In case you want to suppress such behaviour, which should be very rare +case, you can use virAspritnfQuiet() or virVasprintfQuiet(). + When printing to a string, consider using virBuffer for incremental allocations, virAsprintf for a one-shot allocation, and snprintf for fixed-width buffers. Do not use sprintf, even if you can prove the buffer diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 1263607..a9b78ba 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -973,6 +973,12 @@ </p> <p> + Moreover, this function automatically reports out-of-memory error if + there's any. In case you want to suppress such behaviour, which should be + very rare case, you can use virAspritnfQuiet() or virVasprintfQuiet(). + </p> + + <p> When printing to a string, consider using virBuffer for incremental allocations, virAsprintf for a one-shot allocation, and snprintf for fixed-width buffers. Do not use sprintf, even diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4cb4a3..8c1d3ab 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1747,6 +1747,7 @@ virStorageFileResize; # util/virstring.h virArgvToString; virAsprintf; +virAsprintfQuiet; virSkipSpaces; virSkipSpacesAndBackslash; virSkipSpacesBackwards; @@ -1767,6 +1768,7 @@ virStrToLong_ul; virStrToLong_ull; virTrimSpaces; virVasprintf; +virVasprintfQuiet; # util/virsysinfo.h diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 5dd4e1f..f05547e 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -96,10 +96,8 @@ void virAuditSend(const char *filename, #endif va_start(args, fmt); - if (virVasprintf(&str, fmt, args) < 0) { + if (virVasprintfQuiet(&str, fmt, args) < 0) VIR_WARN("Out of memory while formatting audit message"); - str = NULL; - } va_end(args); if (auditlog && str) { diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 460e25b..b611048 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1163,7 +1163,7 @@ virCommandAddEnvFormat(virCommandPtr cmd, const char *format, ...) return; va_start(list, format); - if (virVasprintf(&env, format, list) < 0) { + if (virVasprintfQuiet(&env, format, list) < 0) { cmd->has_error = ENOMEM; va_end(list); return; @@ -1377,7 +1377,7 @@ virCommandAddArgFormat(virCommandPtr cmd, const char *format, ...) return; va_start(list, format); - if (virVasprintf(&arg, format, list) < 0) { + if (virVasprintfQuiet(&arg, format, list) < 0) { cmd->has_error = ENOMEM; va_end(list); return; diff --git a/src/util/virerror.c b/src/util/virerror.c index 8761a72..2b3a910 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -650,7 +650,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, } else { va_list ap; va_start(ap, fmt); - ignore_value(virVasprintf(&str, fmt, ap)); + virVasprintfQuiet(&str, fmt, ap); va_end(ap); } diff --git a/src/util/virlog.c b/src/util/virlog.c index 921ffc5..cc3cfa7 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -828,7 +828,7 @@ virLogVMessage(virLogSource source, /* * serialize the error message, add level and timestamp */ - if (virVasprintf(&str, fmt, vargs) < 0) { + if (virVasprintfQuiet(&str, fmt, vargs) < 0) { goto cleanup; } diff --git a/src/util/virstring.c b/src/util/virstring.c index 2f672bc..2416c43 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -325,12 +325,12 @@ virStrToDouble(char const *s, } /** - * virVasprintf + * virVasprintfQuiet * * like glibc's vasprintf but makes sure *strp == NULL on failure */ int -virVasprintf(char **strp, const char *fmt, va_list list) +virVasprintfQuiet(char **strp, const char *fmt, va_list list) { int ret; @@ -341,9 +341,27 @@ virVasprintf(char **strp, const char *fmt, va_list list) } /** + * virVasprintf + * + * like glibc's vasprintf but makes sure *strp == NULL on failure and reports + * an OOM error. + */ +int +virVasprintf(char **strp, const char *fmt, va_list list) +{ + int ret; + + if ((ret = virVasprintfQuiet(strp, fmt, list)) < 0) + virReportOOMError(); + + return ret; +} + +/** * virAsprintf * - * like glibc's_asprintf but makes sure *strp == NULL on failure + * like glibc's_asprintf but makes sure *strp == NULL on failure and reports + * and OOM error. */ int virAsprintf(char **strp, const char *fmt, ...) @@ -352,7 +370,25 @@ virAsprintf(char **strp, const char *fmt, ...) int ret; va_start(ap, fmt); - ret = virVasprintf(strp, fmt, ap); + /* XXX Don't report OOM error for now, unless all code is adapted. */ + ret = virVasprintfQuiet(strp, fmt, ap); + va_end(ap); + return ret; +} + +/** + * virAsprintfQuiet + * + * like glibc's_asprintf but makes sure *strp == NULL on failure + */ +int +virAsprintfQuiet(char **strp, const char *fmt, ...) +{ + va_list ap; + int ret; + + va_start(ap, fmt); + ret = virVasprintfQuiet(strp, fmt, ap); va_end(ap); return ret; } diff --git a/src/util/virstring.h b/src/util/virstring.h index 620efba..908e13a 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -79,9 +79,13 @@ void virSkipSpacesBackwards(const char *str, char **endp) int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3) ATTRIBUTE_RETURN_CHECK; +int virAsprintfQuiet(char **strp, const char *fmt, ...) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3); int virVasprintf(char **strp, const char *fmt, va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0) ATTRIBUTE_RETURN_CHECK; +int virVasprintfQuiet(char **strp, const char *fmt, va_list list) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0); char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) ATTRIBUTE_RETURN_CHECK; char *virStrcpy(char *dest, const char *src, size_t destbytes) diff --git a/tools/virsh.c b/tools/virsh.c index de2e57a..a48f27b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2217,7 +2217,7 @@ vshDebug(vshControl *ctl, int level, const char *format, ...) va_end(ap); va_start(ap, format); - if (virVasprintf(&str, format, ap) < 0) { + if (virVasprintfQuiet(&str, format, ap) < 0) { /* Skip debug messages on low memory */ va_end(ap); return; @@ -2237,7 +2237,7 @@ vshPrintExtra(vshControl *ctl, const char *format, ...) return; va_start(ap, format); - if (virVasprintf(&str, format, ap) < 0) { + if (virVasprintfQuiet(&str, format, ap) < 0) { vshError(ctl, "%s", _("Out of memory")); va_end(ap); return; @@ -2269,7 +2269,7 @@ vshError(vshControl *ctl, const char *format, ...) va_start(ap, format); /* We can't recursively call vshError on an OOM situation, so ignore failure here. */ - ignore_value(virVasprintf(&str, format, ap)); + virVasprintfQuiet(&str, format, ap); va_end(ap); fprintf(stderr, "%s\n", NULLSTR(str)); -- 1.8.1.5 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list