[PATCH 1/9] string-list: add string_list_{pop, last} functions
Add a few functions to allow a string-list to be used as a stack:
- string_list_last() lets a caller peek the string_list_item at the
end of the string list. The caller needs to be aware that it is
borrowing a pointer, which can become invalid if/when the
string_list is resized.
- string_list_pop() removes the string_list_item at the end of
the string list.
- there is no string_list_push(); string_list_append() can be used
in its place.
You can use them in this pattern:
while (list.nr) {
struct string_list_item *item = string_list_last();
work_on(item);
string_list_pop(, free_util);
}
Helped-by: Junio C Hamano
Signed-off-by: Stefan Beller
---
string-list.c | 14 ++
string-list.h | 15 +++
2 files changed, 29 insertions(+)
diff --git a/string-list.c b/string-list.c
index 771c4550980..04db2b537c0 100644
--- a/string-list.c
+++ b/string-list.c
@@ -80,6 +80,20 @@ void string_list_remove(struct string_list *list, const char
*string,
}
}
+void string_list_pop(struct string_list *list, int free_util)
+{
+ if (list->nr == 0)
+ BUG("tried to remove an item from empty string list");
+
+ if (list->strdup_strings)
+ free(list->items[list->nr - 1].string);
+
+ if (free_util)
+ free(list->items[list->nr - 1].util);
+
+ list->nr--;
+}
+
int string_list_has_string(const struct string_list *list, const char *string)
{
int exact_match;
diff --git a/string-list.h b/string-list.h
index ff8f6094a33..15f2936e337 100644
--- a/string-list.h
+++ b/string-list.h
@@ -191,6 +191,21 @@ extern void string_list_remove(struct string_list *list,
const char *string,
*/
struct string_list_item *string_list_lookup(struct string_list *list, const
char *string);
+/**
+ * Removes the last item from the list.
+ * The caller must ensure that the list is not empty.
+ */
+void string_list_pop(struct string_list *list, int free_util);
+
+/*
+ * Returns the last item of the list. As it returns the raw access, do not
+ * modify the list while holding onto the returned pointer.
+ */
+static inline struct string_list_item *string_list_last(struct string_list
*list)
+{
+ return >items[list->nr - 1];
+}
+
/*
* Remove all but the first of consecutive entries with the same
* string value. If free_util is true, call free() on the util
--
2.19.0.397.gdd90340f6a-goog
Re: [PATCH 1/9] string-list: add string_list_{pop, last} functions
Stefan Beller writes:
> Add a few functions to allow a string-list to be used as a stack:
>
> - string_list_last() lets a caller peek the string_list_item at the
>end of the string list. The caller needs to be aware that it is
>borrowing a pointer, which can become invalid if/when the
>string_list is resized.
>
> - string_list_pop() removes the string_list_item at the end of
>the string list.
>
> - _pop usually has a friend _push. This role is taken by
> string_list_append already, as they are not symmetrical
> in our code base: _append returns the pointer, such that
> adding a util is easy, but _pop doesn't return such a pointer.
This third-item somehow is indented one place too deeply.
"our pop() does not return a pointer, so we won't introduce push()
that does return a pointer"? Am I reading the third item right?
pop() and push() usually go together, but they are opposite
operations, they are expected to behave oppositely, and they are
expected to be interfaced differently. pop() that does not return a
pointer is no better or no worse match to push() that returns a
pointer, no?
Lack of something_push(), when something_pop() exists, would always
be surprising to anybody new to the "something" API, and no amount
of mumbling would justify it, I would think, but this third item
sounds like it is trying to make a lame excuse when there is no need
to.
Wouldn't it be sufficient to say "there is no string_list_push();
string_list_append() can be used in its place" and stop there?
> You can use them in this pattern:
>
> while (list.nr) {
> struct string_list_item *item = string_list_last();
>
> work_on(item);
> string_list_pop();
> }
"free_util" as the second argument to this sample call is missing.
> string-list.c | 14 ++
> string-list.h | 11 +++
> 2 files changed, 25 insertions(+)
> diff --git a/string-list.c b/string-list.c
> index 771c4550980..04db2b537c0 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -80,6 +80,20 @@ void string_list_remove(struct string_list *list, const
> char *string,
> }
> }
>
> +void string_list_pop(struct string_list *list, int free_util)
> +{
> + if (list->nr == 0)
> + BUG("tried to remove an item from empty string list");
> +
> + if (list->strdup_strings)
> + free(list->items[list->nr - 1].string);
> +
> + if (free_util)
> + free(list->items[list->nr - 1].util);
> +
> + list->nr--;
> +}
> +
> int string_list_has_string(const struct string_list *list, const char
> *string)
> {
> int exact_match;
> diff --git a/string-list.h b/string-list.h
> index ff8f6094a33..ce2528bbe15 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -191,6 +191,17 @@ extern void string_list_remove(struct string_list *list,
> const char *string,
> */
> struct string_list_item *string_list_lookup(struct string_list *list, const
> char *string);
>
> +/**
> + * Removes the last item from the list.
> + * The caller must ensure that the list is not empty.
> + */
> +void string_list_pop(struct string_list *list, int free_util);
> +
> +static inline struct string_list_item *string_list_last(struct string_list
> *list)
We may want to warn users that pop(), append(), etc. shouldn't be
done while using the returned value from this function in an in-code
comment or docstring.
> +{
> + return >items[list->nr - 1];
> +}
> +
Other than that, nicely done.
> /*
> * Remove all but the first of consecutive entries with the same
> * string value. If free_util is true, call free() on the util
Re: [PATCH 1/9] string-list: add string_list_{pop, last} functions
On 12/09/18 00:49, Stefan Beller wrote:
> Add a few functions to allow a string-list to be used as a stack:
>
> - string_list_last() lets a caller peek the string_list_item at the
>end of the string list. The caller needs to be aware that it is
>borrowing a pointer, which can become invalid if/when the
>string_list is resized.
>
> - string_list_pop() removes the string_list_item at the end of
>the string list.
>
> - _pop usually has a friend _push. This role is taken by
> string_list_append already, as they are not symmetrical
> in our code base: _append returns the pointer, such that
> adding a util is easy, but _pop doesn't return such a pointer.
>
> You can use them in this pattern:
>
> while (list.nr) {
> struct string_list_item *item = string_list_last();
>
> work_on(item);
> string_list_pop();
string_list_pop() takes a second int parameter (free_util).
ATB,
Ramsay Jones
[PATCH 1/9] string-list: add string_list_{pop, last} functions
Add a few functions to allow a string-list to be used as a stack:
- string_list_last() lets a caller peek the string_list_item at the
end of the string list. The caller needs to be aware that it is
borrowing a pointer, which can become invalid if/when the
string_list is resized.
- string_list_pop() removes the string_list_item at the end of
the string list.
- _pop usually has a friend _push. This role is taken by
string_list_append already, as they are not symmetrical
in our code base: _append returns the pointer, such that
adding a util is easy, but _pop doesn't return such a pointer.
You can use them in this pattern:
while (list.nr) {
struct string_list_item *item = string_list_last();
work_on(item);
string_list_pop();
}
Helped-by: Junio C Hamano
Signed-off-by: Stefan Beller
---
string-list.c | 14 ++
string-list.h | 11 +++
2 files changed, 25 insertions(+)
diff --git a/string-list.c b/string-list.c
index 771c4550980..04db2b537c0 100644
--- a/string-list.c
+++ b/string-list.c
@@ -80,6 +80,20 @@ void string_list_remove(struct string_list *list, const char
*string,
}
}
+void string_list_pop(struct string_list *list, int free_util)
+{
+ if (list->nr == 0)
+ BUG("tried to remove an item from empty string list");
+
+ if (list->strdup_strings)
+ free(list->items[list->nr - 1].string);
+
+ if (free_util)
+ free(list->items[list->nr - 1].util);
+
+ list->nr--;
+}
+
int string_list_has_string(const struct string_list *list, const char *string)
{
int exact_match;
diff --git a/string-list.h b/string-list.h
index ff8f6094a33..ce2528bbe15 100644
--- a/string-list.h
+++ b/string-list.h
@@ -191,6 +191,17 @@ extern void string_list_remove(struct string_list *list,
const char *string,
*/
struct string_list_item *string_list_lookup(struct string_list *list, const
char *string);
+/**
+ * Removes the last item from the list.
+ * The caller must ensure that the list is not empty.
+ */
+void string_list_pop(struct string_list *list, int free_util);
+
+static inline struct string_list_item *string_list_last(struct string_list
*list)
+{
+ return >items[list->nr - 1];
+}
+
/*
* Remove all but the first of consecutive entries with the same
* string value. If free_util is true, call free() on the util
--
2.19.0.397.gdd90340f6a-goog
