Re: [PATCH 1/4] Add a new function, string_list_split_in_place()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> ...  Consider something like
>
>     struct string_list *split_file_into_words(FILE *f)
>     {
>         char buf[1024];
>         struct string_list *list = new string list;
>         list->strdup_strings = 1;
>         while (not EOF) {
>             read_line_into_buf();
>             string_list_split_in_place(list, buf, ' ', -1);
>         }
>         return list;
>     }

That is a prime example to argue that string_list_split() would make
more sense, no?  The caller does _not_ mind if the function mucks
with buf, but the resulting list is not allowed to point into buf.

In such a case, the caller shouldn't have to _care_ if it wants to
allow buf to be mucked with; it is already asking that the resulting
list _not_ point into buf by setting strdup_strings (by the way,
that is part of the function input, so think of it like various *opt
variables passed into functions to tweak their behaviour).  If the
implementation can do so without sacrificing performance (and in
this case, as you said, it can), it should take "const char *buf".

The above caller shouldn't have to choose between sl_split() and
sl_split_in_place(), in other words.

So it appears to me that sl_split_in_place(), if implemented, should
be kept as a special case for performance-minded callers that have
full control of the lifetime rules of the variables they use, can
set strdup_strings to false, and can let buf modified in place, and
can accept list that point into buf.

>>> + * Examples:
>>> + *   string_list_split_in_place(l, "foo:bar:baz", ':', -1) -> ["foo", "bar", "baz"]
>>> + *   string_list_split_in_place(l, "foo:bar:baz", ':', 1) -> ["foo", "bar:baz"]
>>> + *   string_list_split_in_place(l, "foo:bar:", ':', -1) -> ["foo", "bar", ""]
>> 
>> I would find it more natural to see a sentinel value against
>> "positive" to be 0, not -1.  "-1" gives an impression as if "-2"
>> might do something different from "-1", but Zero is a lot more
>> special.
>
> You have raised a good point and I think there is a flaw in the API, but
> I'm not sure I agree with you what the flaw is...
>
> The "maxsplit" argument limits the number of times the string should be
> split.  I.e., if maxsplit is set, then the output will have at most
> (maxsplit + 1) strings.

So "do not split, just give me the whole thing" would be maxsplit == 0
to split into (maxsplit+1) == 1 string.  I think we are in agreement
that your "-1" does not make any sense, and your documentation that
said "positive" is the saner thing to do, no?
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]