From 9524c7eb8e54697da5a366c8ad3405f75c3f4195 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 4 Apr 2024 14:04:09 +1030 Subject: [PATCH] tal: fix clang complaining we return NULL if we promised not to. Since really, the user is promising their alloc fn won't return NULL. Signed-off-by: Rusty Russell --- ccan/tal/str/str.c | 45 ++++++++-------------------------------- ccan/tal/str/str.h | 51 ++++++++++++++++++++++++---------------------- ccan/tal/tal.c | 17 +++++++++++++--- 3 files changed, 49 insertions(+), 64 deletions(-) diff --git a/ccan/tal/str/str.c b/ccan/tal/str/str.c index cec31c75..617b942c 100644 --- a/ccan/tal/str/str.c +++ b/ccan/tal/str/str.c @@ -14,21 +14,14 @@ char *tal_strdup_(const tal_t *ctx, const char *p, const char *label) { - /* We have to let through NULL for take(). */ - return tal_dup_arr_label(ctx, char, p, p ? strlen(p) + 1: 1, 0, label); + return tal_dup_arr_label(ctx, char, p, strlen(p) + 1, 0, label); } char *tal_strndup_(const tal_t *ctx, const char *p, size_t n, const char *label) { - size_t len; + size_t len = strnlen(p, n); char *ret; - /* We have to let through NULL for take(). */ - if (likely(p)) - len = strnlen(p, n); - else - len = n; - ret = tal_dup_arr_label(ctx, char, p, len, 1, label); if (ret) ret[len] = '\0'; @@ -84,9 +77,6 @@ char *tal_vfmt_(const tal_t *ctx, const char *fmt, va_list ap, const char *label { char *buf; - if (!fmt && taken(fmt)) - return NULL; - /* A decent guess to start. */ buf = tal_arr_label(ctx, char, strlen(fmt) * 2, label); if (!do_vfmt(&buf, 0, fmt, ap)) @@ -96,9 +86,6 @@ char *tal_vfmt_(const tal_t *ctx, const char *fmt, va_list ap, const char *label bool tal_append_vfmt(char **baseptr, const char *fmt, va_list ap) { - if (!fmt && taken(fmt)) - return false; - return do_vfmt(baseptr, strlen(*baseptr), fmt, ap); } @@ -120,13 +107,7 @@ char *tal_strcat_(const tal_t *ctx, const char *s1, const char *s2, size_t len1, len2; char *ret; - if (unlikely(!s2) && taken(s2)) { - if (taken(s1)) - tal_free(s1); - return NULL; - } - /* We have to let through NULL for take(). */ - len1 = s1 ? strlen(s1) : 0; + len1 = strlen(s1); len2 = strlen(s2); ret = tal_dup_arr_label(ctx, char, s1, len1, len2 + 1, label); @@ -151,13 +132,11 @@ char **tal_strsplit_(const tal_t *ctx, tal_free(string); if (taken(delims)) tal_free(delims); - return NULL; + return parts; } str = tal_strdup(parts, string); if (unlikely(!str)) goto fail; - if (unlikely(!delims) && is_taken(delims)) - goto fail; if (flags == STR_NO_EMPTY) str += strspn(str, delims); @@ -185,10 +164,14 @@ char **tal_strsplit_(const tal_t *ctx, return parts; fail: +#ifdef CCAN_TAL_NEVER_RETURN_NULL + abort(); +#else tal_free(parts); if (taken(delims)) tal_free(delims); return NULL; +#endif } char *tal_strjoin_(const tal_t *ctx, @@ -199,12 +182,6 @@ char *tal_strjoin_(const tal_t *ctx, char *ret = NULL; size_t totlen = 0, dlen; - if (unlikely(!strings) && is_taken(strings)) - goto fail; - - if (unlikely(!delim) && is_taken(delim)) - goto fail; - dlen = strlen(delim); ret = tal_arr_label(ctx, char, dlen*2+1, label); if (!ret) @@ -269,15 +246,9 @@ bool tal_strreg_(const tal_t *ctx, const char *string, const char *label, unsigned int i; va_list ap; - if (unlikely(!regex) && is_taken(regex)) - goto fail_no_re; - if (regcomp(&r, regex, REG_EXTENDED) != 0) goto fail_no_re; - if (unlikely(!string) && is_taken(string)) - goto fail; - if (regexec(&r, string, nmatch, matches, 0) != 0) goto fail; diff --git a/ccan/tal/str/str.h b/ccan/tal/str/str.h index a2424cbe..12b16478 100644 --- a/ccan/tal/str/str.h +++ b/ccan/tal/str/str.h @@ -12,18 +12,18 @@ /** * tal_strdup - duplicate a string * @ctx: NULL, or tal allocated object to be parent. - * @p: the string to copy (can be take()). + * @p: the string to copy (can be take(), must not be NULL). * * The returned string will have tal_count() == strlen() + 1. */ #define tal_strdup(ctx, p) tal_strdup_(ctx, p, TAL_LABEL(char, "[]")) char *tal_strdup_(const tal_t *ctx, const char *p TAKES, const char *label) - TAL_RETURN_PTR; + TAL_RETURN_PTR NON_NULL_ARGS(2); /** * tal_strndup - duplicate a limited amount of a string. * @ctx: NULL, or tal allocated object to be parent. - * @p: the string to copy (can be take()). + * @p: the string to copy (can be take(), must not be NULL). * @n: the maximum length to copy. * * Always gives a nul-terminated string, with strlen() <= @n. @@ -32,24 +32,24 @@ char *tal_strdup_(const tal_t *ctx, const char *p TAKES, const char *label) #define tal_strndup(ctx, p, n) tal_strndup_(ctx, p, n, TAL_LABEL(char, "[]")) char *tal_strndup_(const tal_t *ctx, const char *p TAKES, size_t n, const char *label) - TAL_RETURN_PTR; + TAL_RETURN_PTR NON_NULL_ARGS(2); /** * tal_fmt - allocate a formatted string * @ctx: NULL, or tal allocated object to be parent. - * @fmt: the printf-style format (can be take()). + * @fmt: the printf-style format (can be take(), must not be NULL). * * The returned string will have tal_count() == strlen() + 1. */ #define tal_fmt(ctx, ...) \ tal_fmt_(ctx, TAL_LABEL(char, "[]"), __VA_ARGS__) char *tal_fmt_(const tal_t *ctx, const char *label, const char *fmt TAKES, - ...) PRINTF_FMT(3,4) TAL_RETURN_PTR; + ...) PRINTF_FMT(3,4) TAL_RETURN_PTR NON_NULL_ARGS(3); /** * tal_vfmt - allocate a formatted string (va_list version) * @ctx: NULL, or tal allocated object to be parent. - * @fmt: the printf-style format (can be take()). + * @fmt: the printf-style format (can be take(), must not be NULL). * @va: the va_list containing the format args. * * The returned string will have tal_count() == strlen() + 1. @@ -58,40 +58,42 @@ char *tal_fmt_(const tal_t *ctx, const char *label, const char *fmt TAKES, tal_vfmt_(ctx, fmt, va, TAL_LABEL(char, "[]")) char *tal_vfmt_(const tal_t *ctx, const char *fmt TAKES, va_list ap, const char *label) - PRINTF_FMT(2,0) TAL_RETURN_PTR; + PRINTF_FMT(2,0) TAL_RETURN_PTR NON_NULL_ARGS(2); /** * tal_append_fmt - append a formatted string to a talloc string. * @baseptr: a pointer to the tal string to be appended to. - * @fmt: the printf-style format (can be take()). + * @fmt: the printf-style format (can be take(), must not be NULL). * * Returns false on allocation failure. * Otherwise tal_count(*@baseptr) == strlen(*@baseptr) + 1. */ -bool tal_append_fmt(char **baseptr, const char *fmt TAKES, ...) PRINTF_FMT(2,3); +bool tal_append_fmt(char **baseptr, const char *fmt TAKES, ...) + PRINTF_FMT(2,3) NON_NULL_ARGS(2); /** * tal_append_vfmt - append a formatted string to a talloc string (va_list) * @baseptr: a pointer to the tal string to be appended to. - * @fmt: the printf-style format (can be take()). + * @fmt: the printf-style format (can be take(), must not be NULL). * @va: the va_list containing the format args. * * Returns false on allocation failure. * Otherwise tal_count(*@baseptr) == strlen(*@baseptr) + 1. */ -bool tal_append_vfmt(char **baseptr, const char *fmt TAKES, va_list ap); +bool tal_append_vfmt(char **baseptr, const char *fmt TAKES, va_list ap) + NON_NULL_ARGS(2); /** * tal_strcat - join two strings together * @ctx: NULL, or tal allocated object to be parent. - * @s1: the first string (can be take()). - * @s2: the second string (can be take()). + * @s1: the first string (can be take(), must not be NULL). + * @s2: the second string (can be take(), must not be NULL). * * The returned string will have tal_count() == strlen() + 1. */ #define tal_strcat(ctx, s1, s2) tal_strcat_(ctx, s1, s2, TAL_LABEL(char, "[]")) char *tal_strcat_(const tal_t *ctx, const char *s1 TAKES, const char *s2 TAKES, - const char *label) TAL_RETURN_PTR; + const char *label) TAL_RETURN_PTR NON_NULL_ARGS(2,3); enum strsplit { STR_EMPTY_OK, @@ -101,8 +103,8 @@ enum strsplit { /** * tal_strsplit - Split string into an array of substrings * @ctx: the context to tal from (often NULL). - * @string: the string to split (can be take()). - * @delims: delimiters where lines should be split (can be take()). + * @string: the string to split (can be take(), must not be NULL). + * @delims: delimiters where lines should be split (can be take(), must not be NULL). * @flags: whether to include empty substrings. * * This function splits a single string into multiple strings. @@ -140,7 +142,7 @@ char **tal_strsplit_(const tal_t *ctx, const char *delims TAKES, enum strsplit flag, const char *label) - TAL_RETURN_PTR; + TAL_RETURN_PTR NON_NULL_ARGS(2,3); enum strjoin { STR_TRAIL, @@ -150,8 +152,8 @@ enum strjoin { /** * tal_strjoin - Join an array of substrings into one long string * @ctx: the context to tal from (often NULL). - * @strings: the NULL-terminated array of strings to join (can be take()) - * @delim: the delimiter to insert between the strings (can be take()) + * @strings: the NULL-terminated array of strings to join (can be take(), must not be NULL) + * @delim: the delimiter to insert between the strings (can be take(), must not be NULL) * @flags: whether to add a delimieter to the end * * This function joins an array of strings into a single string. The @@ -179,13 +181,13 @@ char *tal_strjoin_(const void *ctx, const char *delim TAKES, enum strjoin flags, const char *label) - TAL_RETURN_PTR; + TAL_RETURN_PTR NON_NULL_ARGS(2,3); /** * tal_strreg - match/extract from a string via (extended) regular expressions. * @ctx: the context to tal from (often NULL) - * @string: the string to try to match (can be take()) - * @regex: the regular expression to match (can be take()) + * @string: the string to try to match (can be take(), must not be NULL) + * @regex: the regular expression to match (can be take(), must not be NULL) * ...: pointers to strings to allocate for subexpressions. * * Returns true if we matched, in which case any parenthesized @@ -225,5 +227,6 @@ char *tal_strjoin_(const void *ctx, #define tal_strreg(ctx, string, ...) \ tal_strreg_(ctx, string, TAL_LABEL(char, "[]"), __VA_ARGS__) bool tal_strreg_(const void *ctx, const char *string TAKES, - const char *label, const char *regex, ...); + const char *label, const char *regex TAKES, ...) + NON_NULL_ARGS(2,4); #endif /* CCAN_STR_TAL_H */ diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c index 1230d8ca..e799059f 100644 --- a/ccan/tal/tal.c +++ b/ccan/tal/tal.c @@ -456,13 +456,24 @@ static void del_tree(struct tal_hdr *t, const tal_t *orig, int saved_errno) freefn(t); } +/* Don't have compiler complain we're returning NULL if we promised not to! */ +static void *null_alloc_failed(void) +{ +#ifdef CCAN_TAL_NEVER_RETURN_NULL + abort(); +#else + return NULL; +#endif /* CCAN_TAL_NEVER_RETURN_NULL */ +} + void *tal_alloc_(const tal_t *ctx, size_t size, bool clear, const char *label) { struct tal_hdr *child, *parent = debug_tal(to_tal_hdr_or_null(ctx)); child = allocate(sizeof(struct tal_hdr) + size); if (!child) - return NULL; + return null_alloc_failed(); + if (clear) memset(from_tal_hdr(child), 0, size); child->prop = (void *)label; @@ -470,7 +481,7 @@ void *tal_alloc_(const tal_t *ctx, size_t size, bool clear, const char *label) if (!add_child(parent, child)) { freefn(child); - return NULL; + return null_alloc_failed(); } debug_tal(parent); if (notifiers) @@ -501,7 +512,7 @@ void *tal_alloc_arr_(const tal_t *ctx, size_t size, size_t count, bool clear, const char *label) { if (!adjust_size(&size, count)) - return NULL; + return null_alloc_failed(); return tal_alloc_(ctx, size, clear, label); } -- 2.39.5