On Tue, Jun 2, 2015 at 8:10 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>> On Tue, Jun 2, 2015 at 7:02 AM, Stefan Beller <sbeller@xxxxxxxxxx> 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!
--
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