Re: [PATCH v1 1/8] lib/string: introduce match_string() helper

[Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>]

Hello,

On (01/11/16 23:10), Rasmus Villemoes wrote:
[..]
> > Thought more about those cases.
> >
> > If you would like you may introduce something like
> >
> > int nmatch_string(array, array_size, string, int len)
> > {
> >   if (len < 0)
> >     return match_string();
> >
> >   for (...) {
> >     size_t itemlen = (len > 0) ? len : strlen(array[index]);
> > ...
> >     if (!strncmp(array[index], string, itemlen))
> >       return index;
> >   }
> >   return -EINVAL;
> > }

may be later this week; I'm a bit out of spare time at the moment.


> Yeah, a separate function is probably better. But why not a more
> explicit name, match_prefix, match_string_prefix, match_string_starts?

Not married to nmatch_string(), but at the same time, IMHO, *_prefix or
*_starts naming is not really better. One can pass a string with offset,
e.g.
  FOO_starts(array, array_size, string + offset, strlen(string) - offset)
which will be equivalent to FOO_ends(), but not FOO_starts() or FOO_prefix().
Personally, I'd prefer to preserve strcmp/strncmp semantics, thus, forbidding
`len < 0' case, which looks cryptic to me.


> I like the idea of passing the string length if one wants the "is this a
> prefix of some array element" semantics, and a sentinel otherwise. But I
> don't see any case where one would want match_string() semantics (why
> not call match_string directly instead?),


> so why not let len < 0 mean "is some array element a prefix of this string"
> and "len >= 0" be the other case. I don't see why one shouldn't be able to
> ask "is the empty string a prefix of some array element" (that is, are there
> any elements in the array);

if this is a dynamic array, then there should be some function that
fills in that array, so having a simple bool flag in the code will
suffice; if this is a static array, then ARRAY_SIZE() should do the
trick. I would never expect a string matching function to have this
type of functionality, TBH.

But the question is

> is the empty string a prefix of some array element

do people really need this?

the way I see it, the idea is to have wrappers around
	while (array[..]) if strcmp()/strncmp() == 0 break ...

both of which [strcmp()/strncmp()] have a well known and expected
semantics, changing this can only confuse people.

	-ss

> both the array and the string might be run-time things,
> so this could occur. And it's not up to a generic library routine like
> this to impose restrictions like "the empty string makes no sense, go
> away".
> 
> Rasmus
>