Willy,
On 4/7/21 7:56 PM, Willy Tarreau wrote:
Overall it all looks good so I've merged it. I'd just have one small
request regarding istappend(), it's the first really unsafe function
we have in this collection that could be used inside a loop and cause
buffer overflows, especially since ist strings are designed to be
easier to use than plain strings (i.e. users care less). I'm prefectly
fine with having unsafe functions but not with a default name, so I'd
rather have __istappend() that the caller knows he wants to use and
takes the responsibility for, and istappend() that adds the length
check against an extra argument "size" as a few other functions do
in this case (e.g. istcat() uses a count argument for this).
No emergency but since I guess you're using them in your code, it would
be nice that your first caller uses either a secured or explicit version.
I'll opt for the explicit version, because for a secured version I'd
need a proper return value to indicate whether it succeeded or not and I
really don't want istappend() to take a pointer. I suspect that no one
would have checked that return value anyway, because it is not really
actionable.
I'd preferred if you would not have taken the patch yet. A follow-up
feels ugly and writing a good commit message is hard :-)
Patch atteched.
Best regards
Tim Düsterhus
>From e0bfbc39ca443ca07dc4676b78ea56d131adb311 Mon Sep 17 00:00:00 2001
From: Tim Duesterhus <[email protected]>
Date: Thu, 8 Apr 2021 19:28:16 +0200
Subject: [PATCH] MINOR: ist: Rename istappend() to __istappend()
To: [email protected]
Cc: [email protected]
Indicate that this function is not inherently safe by adding two underscores as
a prefix.
---
include/import/ist.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/import/ist.h b/include/import/ist.h
index daf23d552..a19e22802 100644
--- a/include/import/ist.h
+++ b/include/import/ist.h
@@ -425,7 +425,7 @@ static inline int istneq(const struct ist ist1, const struct ist ist2, size_t co
/* appends <src> after <dst>. The caller must ensure that the underlying buffer
* is large enough to fit the character.
*/
-static inline struct ist istappend(struct ist dst, const char src)
+static inline struct ist __istappend(struct ist dst, const char src)
{
dst.ptr[dst.len++] = src;
--
2.31.1
