[systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv

Tom Gundersen teg at jklm.no
Mon Feb 16 06:20:21 PST 2015


On Mon, Feb 16, 2015 at 2:54 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek at in.waw.pl> wrote:
> On Mon, Feb 16, 2015 at 02:12:38PM +0100, Lennart Poettering wrote:
>> On Sat, 14.02.15 00:53, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
>>
>> > No functional change intended.
>>
>> I like this simplification!
>>
>> >
>> >          if (match_host && !condition_test(match_host))
>> >                  return false;
>> > @@ -117,49 +112,17 @@ bool net_match_config(const struct ether_addr *match_mac,
>> >          if (match_mac && (!dev_mac || memcmp(match_mac, dev_mac, ETH_ALEN)))
>> >                  return false;
>> >
>> > -        if (!strv_isempty(match_paths)) {
>> > -                if (!dev_path)
>> > -                        return false;
>> > +        if (!strv_isempty(match_paths))
>> > +                return strv_fnmatch(dev_path, match_paths, 0);
>>
>> Can't this be shortened further by combining the stv_isempty() with
>> the strv_fnmatch?
> This code is changed in 2/3. I believe it is broken in the original
> version (and after the change above, which does not change functionality).
>
>
>> > +bool strv_fnmatch(const char *s, char* const* patterns, int flags);
>> > +
>> > +static inline bool strv_fnmatch_or_empty(const char *s, char* const* patterns, int flags) {
>> > +        assert(s);
>> > +        return strv_isempty(patterns) ||
>> > +               strv_fnmatch(s, patterns, flags);
>> > +}
>>
>> Wouldn't the order of arguments be more natural if we specified the
>> strv ("haystack") first, and the string ("needle") second? After all,
>> it's kinda an OO interface, where the first object should come first?
> Yeah, like strv_find and friends. I'll do that.
>
>> Anyway, this all looks like a great simplification. If this doesn't
>> change behaviour I love the idea, please apply!
> I'll wait for some feedback on 2/3 from Tom.

Hm, I haven't received these patches (yet?), care to point me at a
public branch instead?

Cheers,

Tom


More information about the systemd-devel mailing list