Re: [PATCH 2/4] Add a new function, filter_string_list()
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Sep 10, 2012, 08:58 UTC
- Message-ID
- <504DABB5.7090401@alum.mit.edu>
- In-Reply-To
- <7vk3w3sfe9.fsf@alter.siamese.dyndns.org>
On 09/09/2012 11:40 AM, Junio C Hamano wrote:
Show 27 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes: > >> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> >> --- >> Documentation/technical/api-string-list.txt | 8 ++++++++ >> string-list.c | 17 +++++++++++++++++ >> string-list.h | 9 +++++++++ >> 3 files changed, 34 insertions(+) >> >> diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt >> index 3b959a2..15b8072 100644 >> --- a/Documentation/technical/api-string-list.txt >> +++ b/Documentation/technical/api-string-list.txt >> @@ -60,6 +60,14 @@ Functions >> >> * General ones (works with sorted and unsorted lists as well) >> >> +`filter_string_list`:: >> + >> + Apply a function to each item in a list, retaining only the >> + items for which the function returns true. If free_util is >> + true, call free() on the util members of any items that have >> + to be deleted. Preserve the order of the items that are >> + retained. > > In other words, this can safely be used on both sorted and unsorted > string list. Good.
Preserving order (while retaining performance) is the main reason for this function. Otherwise, unsorted_string_list_delete_item() could be used in a loop.
Show 56 quoted lines
>> `print_string_list`::
>>
>> Dump a string_list to stdout, useful mainly for debugging purposes. It
>> diff --git a/string-list.c b/string-list.c
>> index 110449c..72610ce 100644
>> --- a/string-list.c
>> +++ b/string-list.c
>> @@ -102,6 +102,23 @@ int for_each_string_list(struct string_list *list,
>> return ret;
>> }
>>
>> +void filter_string_list(struct string_list *list, int free_util,
>> + string_list_each_func_t fn, void *cb_data)
>> +{
>> + int src, dst = 0;
>> + for (src = 0; src < list->nr; src++) {
>> + if (fn(&list->items[src], cb_data)) {
>> + list->items[dst++] = list->items[src];
>> + } else {
>> + if (list->strdup_strings)
>> + free(list->items[src].string);
>> + if (free_util)
>> + free(list->items[src].util);
>> + }
>> + }
>> + list->nr = dst;
>> +}
>> +
>> void string_list_clear(struct string_list *list, int free_util)
>> {
>> if (list->items) {
>> diff --git a/string-list.h b/string-list.h
>> index 7e51d03..84996aa 100644
>> --- a/string-list.h
>> +++ b/string-list.h
>> @@ -29,6 +29,15 @@ int for_each_string_list(struct string_list *list,
>> #define for_each_string_list_item(item,list) \
>> for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
>>
>> +/*
>> + * Apply fn to each item in list, retaining only the ones for which
>> + * the function returns true. If free_util is true, call free() on
>> + * the util members of any items that have to be deleted. Preserve
>> + * the order of the items that are retained.
>> + */
>> +void filter_string_list(struct string_list *list, int free_util,
>> + string_list_each_func_t fn, void *cb_data);
>> +
>> /* Use these functions only on sorted lists: */
>> int string_list_has_string(const struct string_list *list, const char *string);
>> int string_list_find_insert_index(const struct string_list *list, const char *string,
>
> Having seen that the previous patch introduced a new test helper for
> unit testing (which is a very good idea) and dedicated a new test
> number, I would have expected to see a new test for filtering
> here.I thought that the code was too trivial to warrant a test, especially considering that the memory handling aspect of the function can't be tested very well. But you've correctly shamed me into adding tests for this and also for patch 3/4, string_list_remove_duplicates().
Michael
-- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/