On 09/09/2012 11:40 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>
>> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
>> ---
>> 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.
>> `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@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html