On Mon, Apr 13, 2026 at 07:37:27PM +0200, Tim Duesterhus wrote:
> This new function will handle the actual generation of the unique ID according
> to a format. The caller is responsible to check that no unique ID is stored
> yet.
It's indeed cleaner this way. However for me there's a bug below:
> +/* Generates a unique ID based on the given <format> and stores it
> + * in <dst>. <dst> must be IST_NULL when this function is called.
> + *
> + * If this function fails to allocate memory IST_NULL is stored.
> + */
> +void generate_unique_id(struct ist *dst, struct session *sess, struct stream
> *strm, struct lf_expr *format)
> +{
> + char *unique_id;
> +
> + BUG_ON(isttest(*dst));
> +
> + if ((unique_id = pool_alloc(pool_head_uniqueid)) == NULL) {
> + *dst = IST_NULL;
> + }
I guess you wanted to add a return statement at the end of this block.
BTW, generally speaking, it's better to avoid assignments in "if"
conditions like this as long as possible (sometimes it's *really*
helpful). The point is that it usually complicates debugging, when
you start to add printfs at certain points, you just can't anymore.
And if you quickly comment out an "if" block that handles a particular
error case, you accidentally lose your assignment. I found myself so
many times having to use compound expressions just to add a printf,
despite the extremely low number of such assignments that it's really
not worth it.
> + /* Initialize <dst> to an empty string to prevent infinite
> + * recursion when the <format> references %[unique-id] or %ID.
> + */
> + *dst = ist2(unique_id, 0);
> + dst->len = sess_build_logline(sess, strm, unique_id, UNIQUEID_LEN,
> format);
> +}
> +
> /* Builds a log line in <dst> based on <lf_expr>, and stops before reaching
> * <maxsize> characters. Returns the size of the output string in characters,
> * not counting the trailing zero which is always added if the resulting size
> diff --git a/src/stream.c b/src/stream.c
> index eb13cf277..9e026c23d 100644
> --- a/src/stream.c
> +++ b/src/stream.c
> @@ -3094,16 +3094,7 @@ INITCALL0(STG_INIT, init_stream);
> struct ist stream_generate_unique_id(struct stream *strm, struct lf_expr
> *format)
> {
> if (!isttest(strm->unique_id)) {
> - char *unique_id;
> -
> - if ((unique_id = pool_alloc(pool_head_uniqueid)) == NULL)
> - return IST_NULL;
> -
> - /* Initialize ->unique_id to an empty string to prevent infinite
> - * recursion when the <format> references %[unique-id] or %ID.
> - */
> - strm->unique_id = ist2(unique_id, 0);
> - strm->unique_id.len = build_logline(strm, unique_id,
> UNIQUEID_LEN, format);
> + generate_unique_id(&strm->unique_id, strm_sess(strm), strm,
> format);
> }
Yeah this is cleaner now.
Willy
