On Mon, 2023-08-28 at 21:53 +0200, Pablo Neira Ayuso wrote: > On Mon, Aug 28, 2023 at 06:45:00PM +0200, Thomas Haller wrote: > > On Mon, 2023-08-28 at 18:04 +0200, Pablo Neira Ayuso wrote: > > > On Mon, Aug 28, 2023 at 05:49:53PM +0200, Thomas Haller wrote: > > > > On Mon, 2023-08-28 at 17:13 +0200, Pablo Neira Ayuso wrote: > > > > > > > SNPRINTF_BUFFER_SIZE() rejects truncation of the string by > > > > asserting > > > > against it. That behavior is part of the API of that function. > > > > Error > > > > checking after an assert seems unnecessary. > > > > > > > > The check "if (len == NF_LOG_PREFIXLEN)" seems wrong anyway. > > > > After > > > > truncation, "len" would be zero. The code previously checked > > > > whether > > > > nothing was appended, but the error string didn't match that > > > > situation. > > > > > > > > Maybe SNPRINTF_BUFFER_SIZE() should not assert against > > > > truncation? > > > > > > IIRC, the goal for this function was to handle snprintf() and all > > > its > > > corner cases. If there is no need for it or a better way to do > > > this, > > > this is welcome. > > > > > > > I think the macro is sensible (at least, after some cleanup). > > > > It makes a choice, that the caller must ensure a priori that the > > buffer > > is long enough (by asserting). > > > > By looking at the callers, it's not clear to me, whether the > > callers > > can always ensure that. For meta_key_parse(), it seems the maximum > > string is limited by meta_templates. But for > > stmt_evaluate_log_prefix() > > it may be possible to craft user-input that triggers the assertion, > > isn't it? > > > > Maybe the macro and the callers should anticipate and handle > > truncation? > > This should not silently truncate strings, instead bail out to user > in > case the string is too long. v2 of the patchset is supposed to do that. wdyt? Thanks Thomas
