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

On Thu, Aug 9, 2018 at 2:41 PM Jeff King [off-list ref] wrote:

quoted

      while (list->nr)
              work_on(list_pop(list));

which is not so bad.

In many cases you can just do:

  while (list->nr) {
        work_on(list->items[list->nr - 1]);
        list_remove(list, list->nr - 1);
  }

and then all of those memory ownership issues like:

[...]

just go away. :)

The only complication here is the lack of list_remove(index),
we do have list_remove(string), which internally searches the
item and removes it. Hence I did not want to use it.

Another idea I had was to keep the list immutable (except amending,
just like a constitution ;-) and store an index of how far we got in that
list already. That wastes memory for keeping entries around, but is safe
for memory due to its nature.

Where that falls down is if you really need work_on() to put more items
on the stack, but only after you've removed the current top. But then
writing it out may still be nicer, because it makes it clear you have to
do:

  const char *cur_string = xstrdup(list->items[list->nr-1].string);

Another way would be to use

  string_list_pop(&list, &string_dst, &util_dst);
i.e.
  /* Returns 0 if the dst was filled */
  int (struct string_list *, char **, void**)

as then we do not expose the internals and would not have issues
with reallocs.

if you want the data to live past the removal.

In the code proposed there are no additions (hence no reallocs)
and the need for the data is short lived.

But I can see how the design was just fitting my purpose and
we could come up with some better API.

Thanks,
Stefan