Hi Tim,
On Fri, Apr 03, 2026 at 11:29:00PM +0200, Tim Duesterhus wrote:
> Use the return value of `stream_generate_unique_id()` instead of relying on
> the
> `unique_id` field of `struct stream` when handling the `%ID` log placeholder.
> This also allowed to unique the "stream available" and "stream not available"
> paths.
>
> Reviewed-by: Volker Dusch <[email protected]>
> ---
> src/log.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/src/log.c b/src/log.c
> index 671eaee01..26d78629f 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -5147,20 +5147,19 @@ size_t sess_build_logline_orig(struct session *sess,
> struct stream *s,
> break;
>
> case LOG_FMT_UNIQUEID: // %ID
> + {
> + struct ist unique_id = IST_NULL;
> +
> ret = NULL;
> - if (s) {
> - /* if unique-id was not generated */
> - if (!isttest(s->unique_id) &&
> !lf_expr_isempty(&sess->fe->format_unique_id)) {
> - stream_generate_unique_id(s,
> &sess->fe->format_unique_id);
> - }
> - ret = lf_text_len(tmplog,
> s->unique_id.ptr, s->unique_id.len, maxsize - (tmplog - dst), ctx);
> + if (s &&
> !lf_expr_isempty(&sess->fe->format_unique_id)) {
> + unique_id =
> stream_generate_unique_id(s, &sess->fe->format_unique_id);
> }
> - else
> - ret = lf_text_len(tmplog, NULL, 0,
> maxsize - (tmplog - dst), ctx);
> + ret = lf_text_len(tmplog, istptr(unique_id),
> istlen(unique_id), maxsize - (tmplog - dst), ctx);
> if (ret == NULL)
> goto out;
> tmplog = ret;
> break;
> + }
While I'm fine with the cleanup, I'm not fond of systematically adding
one function call to retrieve a pointer. How about this almost identical
but cheaper variant?
struct ist unique_id = s ? s->unique_id : IST_NULL;
ret = NULL;
if (s && !ist_test(unique_id) &&
!lf_expr_isempty(&sess->fe->format_unique_id))
unique_id = stream_generate_unique_id(s,
&sess->fe->format_unique_id);
ret = lf_text_len(tmplog, istptr(unique_id), istlen(unique_id), maxsize -
(tmplog - dst), ctx);
Willy
