Re: [PATCH 4/7] util: make string functions abort on OOM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/29/19 12:02 PM, Daniel P. Berrangé  wrote:
> The functions are left returning an "int" to avoid an immediate
> big-bang cleanup. They'll simply never return anything other
> than 0.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>   src/util/virstring.c | 93 +++++++++++---------------------------------
>   src/util/virstring.h | 79 +++++++++++++------------------------
>   2 files changed, 49 insertions(+), 123 deletions(-)
> 
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index bd269e98fe..2064944b0b 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -726,40 +726,27 @@ virDoubleToStr(char **strp, double number)
>   
>   
>   int
> -virVasprintfInternal(bool report,
> -                     int domcode,
> -                     const char *filename,
> -                     const char *funcname,
> -                     size_t linenr,
> -                     char **strp,
> +virVasprintfInternal(char **strp,
>                        const char *fmt,
>                        va_list list)
>   {
>       int ret;
>   
> -    if ((ret = vasprintf(strp, fmt, list)) == -1) {
> -        if (report)
> -            virReportOOMErrorFull(domcode, filename, funcname, linenr);
> -        *strp = NULL;
> -    }
> +    if ((ret = vasprintf(strp, fmt, list)) == -1)
> +        abort();
> +
>       return ret;
>   }
>   
>   int
> -virAsprintfInternal(bool report,
> -                    int domcode,
> -                    const char *filename,
> -                    const char *funcname,
> -                    size_t linenr,
> -                    char **strp,
> +virAsprintfInternal(char **strp,
>                       const char *fmt, ...)
>   {
>       va_list ap;
>       int ret;
>   
>       va_start(ap, fmt);
> -    ret = virVasprintfInternal(report, domcode, filename,
> -                               funcname, linenr, strp, fmt, ap);
> +    ret = virVasprintfInternal(strp, fmt, ap);
>       va_end(ap);
>       return ret;
>   }
> @@ -937,37 +924,20 @@ virStringIsEmpty(const char *str)
>    * virStrdup:
>    * @dest: where to store duplicated string
>    * @src: the source string to duplicate
> - * @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 strdup, 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 virStrdup is called from. Consider
> - * using VIR_STRDUP which sets these automatically.
> - *
> - * Returns: 0 for NULL src, 1 on successful copy, -1 otherwise.
> + *
> + * Wrapper over strdup, which aborts on OOM error.
> + *
> + * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM
>    */
>   int
>   virStrdup(char **dest,
> -          const char *src,
> -          bool report,
> -          int domcode,
> -          const char *filename,
> -          const char *funcname,
> -          size_t linenr)
> +          const char *src)
>   {
>       *dest = NULL;
>       if (!src)
>           return 0;
> -    if (!(*dest = strdup(src))) {
> -        if (report)
> -            virReportOOMErrorFull(domcode, filename, funcname, linenr);
> -        return -1;
> -    }
> +    if (!(*dest = strdup(src)))
> +        abort();
>   
>       return 1;
>   }
> @@ -977,45 +947,28 @@ virStrdup(char **dest,
>    * @dest: where to store duplicated string
>    * @src: the source string to duplicate
>    * @n: how many bytes to copy
> - * @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 strndup, 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 virStrndup is called from. Consider
> - * using VIR_STRNDUP which sets these automatically.
> + *
> + * Wrapper over strndup, which aborts on OOM error.
>    *
>    * In case @n is smaller than zero, the whole @src string is
>    * copied.
>    *
> - * Returns: 0 for NULL src, 1 on successful copy, -1 otherwise.
> + * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM
>    */
>   int
>   virStrndup(char **dest,
>              const char *src,
> -           ssize_t n,
> -           bool report,
> -           int domcode,
> -           const char *filename,
> -           const char *funcname,
> -           size_t linenr)
> +           ssize_t n)
>   {
>       *dest = NULL;
>       if (!src)
>           return 0;
>       if (n < 0)
>           n = strlen(src);
> -    if (!(*dest = strndup(src, n))) {
> -        if (report)
> -            virReportOOMErrorFull(domcode, filename, funcname, linenr);
> -        return -1;
> -    }
> +    if (!(*dest = strndup(src, n)))
> +        abort();
>   
> -   return 1;
> +    return 1;
>   }
>   
>   
> @@ -1483,10 +1436,8 @@ virStringEncodeBase64(const uint8_t *buf, size_t buflen)
>       char *ret;
>   
>       base64_encode_alloc((const char *) buf, buflen, &ret);
> -    if (!ret) {
> -        virReportOOMError();
> -        return NULL;
> -    }
> +    if (!ret)
> +        abort();
>   
>       return ret;
>   }
> diff --git a/src/util/virstring.h b/src/util/virstring.h
> index 5e64ad1bb9..7d1ae1306b 100644
> --- a/src/util/virstring.h
> +++ b/src/util/virstring.h
> @@ -128,23 +128,15 @@ int virStrcpy(char *dest, const char *src, size_t destbytes)
>   #define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest))
>   
>   /* 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);
> -
> -int virStrndup(char **dest, const char *src, ssize_t n, bool report, int domcode,
> -               const char *filename, const char *funcname, size_t linenr)
> -    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
> -int virAsprintfInternal(bool report, int domcode, const char *filename,
> -                        const char *funcname, size_t linenr, char **strp,
> -                        const char *fmt, ...)
> -    ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7) ATTRIBUTE_FMT_PRINTF(7, 8)
> -    ATTRIBUTE_RETURN_CHECK;
> -int virVasprintfInternal(bool report, int domcode, const char *filename,
> -                         const char *funcname, size_t linenr, char **strp,
> -                         const char *fmt, va_list list)
> -    ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(7) ATTRIBUTE_FMT_PRINTF(7, 0)
> -    ATTRIBUTE_RETURN_CHECK;
> +int virStrdup(char **dest, const char *src)
> +    ATTRIBUTE_NONNULL(1);
> +
> +int virStrndup(char **dest, const char *src, ssize_t n)
> +    ATTRIBUTE_NONNULL(1);
> +int virAsprintfInternal(char **strp, const char *fmt, ...)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
> +int virVasprintfInternal(char **strp, const char *fmt, va_list list)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0);
>   
>   /**
>    * VIR_STRDUP:
> @@ -155,11 +147,9 @@ int virVasprintfInternal(bool report, int domcode, const char *filename,
>    *
>    * 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
> + * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
>    */
> -#define VIR_STRDUP(dst, src) virStrdup(&(dst), src, true, VIR_FROM_THIS, \
> -                                       __FILE__, __FUNCTION__, __LINE__)
> +#define VIR_STRDUP(dst, src) virStrdup(&(dst), src)
>   
>   /**
>    * VIR_STRDUP_QUIET:
> @@ -170,9 +160,9 @@ int virVasprintfInternal(bool report, int domcode, const char *filename,
>    *
>    * 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
> + * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
>    */
> -#define VIR_STRDUP_QUIET(dst, src) virStrdup(&(dst), src, false, 0, NULL, NULL, 0)
> +#define VIR_STRDUP_QUIET(dst, src) VIR_STRDUP(dst, src)
>   
>   /**
>    * VIR_STRNDUP:
> @@ -187,12 +177,9 @@ int virVasprintfInternal(bool report, int domcode, const char *filename,
>    *
>    * 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
> + * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
>    */
> -#define VIR_STRNDUP(dst, src, n) virStrndup(&(dst), src, n, true, \
> -                                            VIR_FROM_THIS, __FILE__, \
> -                                            __FUNCTION__, __LINE__)
> +#define VIR_STRNDUP(dst, src, n) virStrndup(&(dst), src, n)
>   
>   /**
>    * VIR_STRNDUP_QUIET:
> @@ -208,51 +195,41 @@ int virVasprintfInternal(bool report, int domcode, const char *filename,
>    *
>    * 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
> + * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
>    */
> -#define VIR_STRNDUP_QUIET(dst, src, n) virStrndup(&(dst), src, n, false, \
> -                                                  0, NULL, NULL, 0)
> +#define VIR_STRNDUP_QUIET(dst, src, n) virStrndup(&(dst), src, n)
>   
>   size_t virStringListLength(const char * const *strings);
>   
>   /**
>    * virVasprintf
>    *
> - * Like glibc's vasprintf but makes sure *strp == NULL on failure, in which
> - * case the OOM error is reported too.
> + * Like glibc's vasprintf but aborts on OOM
>    *
> - * Returns -1 on failure (with OOM error reported), number of bytes printed
> - * on success.
> + * Returns number of bytes printed on success, aborts on OOM
>    */
> -#define virVasprintf(strp, fmt, list) \
> -    virVasprintfInternal(true, VIR_FROM_THIS, __FILE__, __FUNCTION__, \
> -                         __LINE__, strp, fmt, list)
> +#define virVasprintf(strp, fmt, list) virVasprintfInternal(strp, fmt, list)
>   
>   /**
>    * virVasprintfQuiet
>    *
> - * Like glibc's vasprintf but makes sure *strp == NULL on failure.
> + * Like glibc's vasprintf but aborts on OOM.
>    *
> - * Returns -1 on failure, number of bytes printed on success.
> + * Returns number of bytes printed on success, aborts on OOM
>    */
> -#define virVasprintfQuiet(strp, fmt, list) \
> -    virVasprintfInternal(false, 0, NULL, NULL, 0, strp, fmt, list)
> +#define virVasprintfQuiet(strp, fmt, list) virVasprintf(strp, fmt, list)
>   
>   /**
>    * virAsprintf:
>    * @strp: variable to hold result (char **)
>    * @fmt: printf format
>    *
> - * Like glibc's asprintf but makes sure *strp == NULL on failure, in which case
> - * the OOM error is reported too.
> + * Like glibc's asprintf but aborts on OOM.
>    *
> - * Returns -1 on failure (with OOM error reported), number of bytes printed
> - * on success.
> + * Returns number of bytes printed on success, aborts on OOM
>    */
>   
> -#define virAsprintf(strp, ...) \
> -    virAsprintfInternal(true, VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__, \
> -                        strp, __VA_ARGS__)
> +#define virAsprintf(strp, ...) virAsprintfInternal(strp, __VA_ARGS__)
>   
>   /**
>    * virAsprintfQuiet:
> @@ -264,9 +241,7 @@ size_t virStringListLength(const char * const *strings);
>    * Returns -1 on failure, number of bytes printed on success.

You missed changing the comments for virAsprintfQuiet.

Regards,
Jim

>    */
>   
> -#define virAsprintfQuiet(strp, ...) \
> -    virAsprintfInternal(false, 0, NULL, NULL, 0, \
> -                        strp, __VA_ARGS__)
> +#define virAsprintfQuiet(strp, ...) virAsprintf(strp, __VA_ARGS__)
>   
>   int virStringSortCompare(const void *a, const void *b);
>   int virStringSortRevCompare(const void *a, const void *b);
> 


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux