Re: [RFCv2 01/16] stringlist: add from_space_separated_string
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jun 2, 2015, 15:10 UTC
- Message-ID
- <CAPig+cTsBU2t=NJEe16N2CMyYrWgbP-NvoKxNQ9QdPiokXj+hw@mail.gmail.com>
- In-Reply-To
- <CACsJy8BEEvgP_YzVnbPaLL9QBs48sd2fn6CeicXD_9U92=9UnA@mail.gmail.com>
On Tue, Jun 2, 2015 at 5:42 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 15 quoted lines
> 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).