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

[Zbigniew Jędrzejewski-Szmek]

On Mon, Feb 16, 2015 at 06:47:32PM +0100, Tom Gundersen wrote:
> On Mon, Feb 16, 2015 at 3:51 PM, Zbigniew Jędrzejewski-Szmek
> <zbyszek at in.waw.pl> wrote:
> > On Mon, Feb 16, 2015 at 03:20:21PM +0100, Tom Gundersen wrote:
> >> 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?
> > http://lists.freedesktop.org/archives/systemd-devel/2015-February/028350.html
> 
> Thanks Zbigniew,
> 
> So this looks really nice. Looking through it I see a bug in the
> logic, but that was not your fault, so I can just fix that on top.
> Please push.
> 
> FTR, instead of
> 
> return strv_fnmatch(dev_name, match_names, 0);
> 
> we should have
> 
> if (!strv_fnmatch(dev_name, match_names, 0))
>        return false;
That is (almost) what patch 2/3 does, please have a look at ee5de57b9d.

Zbyszek