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 > <[email protected]> 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 > >> <[email protected]> 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 ([email protected]) > >> >> 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 _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
