Re: [RFCv2 01/16] stringlist: add from_space_separated_string

[Stefan Beller <sbeller@google.com>]

On Tue, Jun 2, 2015 at 8:10 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller <sbeller@google.com> wrote:
>>> diff --git a/string-list.h b/string-list.h
>>> index d3809a1..88c18e9 100644
>>> --- a/string-list.h
>>> +++ b/string-list.h
>>> @@ -19,6 +19,7 @@ struct string_list {
>>>  #define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1, NULL }
>>>
>>>  void string_list_init(struct string_list *list, int strdup_strings);
>>> +void from_space_separated_string(struct string_list *list, char *line);
>>
>> The name feels out of place. All functions in here have "string_list"
>> somewhere in their names. The implementation looks very close to
>> string_list_split() but that name's already taken.. Maybe
>> string_list_split_by_space()?
>
> Indeed. If you really want to go the specialized route, splitting only
> on whitespace, then Duy's suggestion makes sense. Alternately,
> string_list_split_ws() might be easily understood while still
> remaining somewhat terse.
>
> However, why make this so specialized? A more generalized function
> could be more widely useful. For instance, you could introduce a
> function very similar to string_list_split() to which you supply
> multiple delimiter characters (as a 'const char *') rather than the
> single delimiter character accepted by string_list_split(). The
> function could be named string_list_split_any() or
> string_list_tokenize().
>
> Also, it's ugly and inconvenient to require the incoming string be
> non-const, and feels as if you're letting the interface be dictated by
> an implementation detail (underlying use of strtok_r).

I see. I think I can even use string_list_split here, and drop this patch.

Thanks for pointing out the flaws!