Re: [PATCH 02/10] string-list.h: add string_list_pop function.

From: Martin Ågren <martin.agren@gmail.com>
Date: Aug 9, 2018, 07:35 UTC
Message-ID: <CAN0heSqjazimYu4de=xCQ9epSxezVyKPQQ16amZZvoaBh_SKdQ@mail.gmail.com>
In-Reply-To: <20180808221752.195419-3-sbeller@google.com>

On 9 August 2018 at 00:17, Stefan Beller <sbeller@google.com> wrote:

Show 21 quoted lines

> A string list can be used as a stack, but should we? A later patch shows
> how useful this will be.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  string-list.c | 8 ++++++++
>  string-list.h | 6 ++++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/string-list.c b/string-list.c
> index 9f651bb4294..ea80afc8a0c 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -80,6 +80,14 @@ void string_list_remove(struct string_list *list, const char *string,
>         }
>  }
>
> +struct string_list_item *string_list_pop(struct string_list *list)
> +{
> +       if (list->nr == 0)
> +               return 0;

return NULL, not 0.

> +
> +       return &list->items[--list->nr];
> +}
> +

Show 6 quoted lines

> +/**
> + * Returns the last item inserted and removes it from the list.
> + * If the list is empty, return NULL.
> + */
> +struct string_list_item *string_list_pop(struct string_list *list);
> +

The memory ownership is now with the caller. That is, the caller needs to check/know `list->strdup_strings` and know `free_util` to be able to properly free all memory.

OTOH, the pointer returned by this function is only guaranteed to be valid until you start inserting into the list (well, you can do one insertion per pop without worrying, but that's quite detailed implementation knowledge).

Maybe these caveats should be documented, or is a hint that this `_pop()` is not so nice to have after all, but let's see what happens in the later patches...

Martin