Subject: Re: [PATCH v2 6/6] trailer: use offsets for trailer_start/trailer_end

From: Junio C Hamano <[email-masked]>
Date: Mon, 11 Sep 2023 12:01:55 -0700

"Linus Arver via GitGitGadget" <[email-masked]> writes:

> From: Linus Arver <[email-masked]>
>
> Previously these fields in the trailer_info struct were of type "const
> char *" and pointed to positions in the input string directly (to the
> start and end positions of the trailer block).
>
> Use offsets to make the intended usage less ambiguous. We only need to
> reference the input string in format_trailer_info(), so update that
> function to take a pointer to the input.

Hmm, I am not sure if this is an improvement.  If the underlying
buffer can be reallocated (to grow), the approach to use the offsets
certainly is easier to deal with, as they will stay valid even after
such a reallocation.  But you lose the obvious sentinel value NULL
that can mean something special, and have to make the readers aware
of the local convention you happened to have picked with a comment
like ...

> Signed-off-by: Linus Arver <[email-masked]>
> ---
>  trailer.c | 17 ++++++++---------
>  trailer.h |  7 +++----
>  2 files changed, 11 insertions(+), 13 deletions(-)
> ...
>  	/*
> -	 * Pointers to the start and end of the trailer block found. If there
> -	 * is no trailer block found, these 2 pointers point to the end of the
> -	 * input string.
> +	 * Offsets to the trailer block start and end positions in the input
> +	 * string. If no trailer block is found, these are set to 0.
>  	 */

... this, simply because there is no obvious sentinel value for an
unsigned integral type; even if you count MAX_ULONG and its friends,
they are not as obvious as NULL for pointer types.

So, I dunno.