Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Mon, Feb 16, 2015 at 07:43:35PM +0100, Lennart Poettering wrote: > On Mon, 16.02.15 19:38, Tom Gundersen (t...@jklm.no) wrote: > > > Perfect! Thanks! > > No, not perfect at all: > > $ ./test-network > Assertion 's' failed at ./src/shared/strv.h:152, function > strv_fnmatch_or_empty(). Aborting. > Aborted (core dumped) Sorry, fixed now. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Mon, 16.02.15 19:38, Tom Gundersen (t...@jklm.no) wrote: > Perfect! Thanks! No, not perfect at all: $ ./test-network Assertion 's' failed at ./src/shared/strv.h:152, function strv_fnmatch_or_empty(). Aborting. Aborted (core dumped) Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
Perfect! Thanks! ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
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
> 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
> >> 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 (zbys...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Mon, Feb 16, 2015 at 3:51 PM, Zbigniew Jędrzejewski-Szmek
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
>> 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 (zbys...@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;
So that we only return true at the very end once all the conditions
have been checked.
Cheers,
Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
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
> 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 (zbys...@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
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Mon, Feb 16, 2015 at 2:54 PM, Zbigniew Jędrzejewski-Szmek
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 (zbys...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Mon, Feb 16, 2015 at 02:12:38PM +0100, Lennart Poettering wrote:
> On Sat, 14.02.15 00:53, Zbigniew Jędrzejewski-Szmek (zbys...@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.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
On Sat, 14.02.15 00:53, Zbigniew Jędrzejewski-Szmek (zbys...@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?
> +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?
Anyway, this all looks like a great simplification. If this doesn't
change behaviour I love the idea, please apply!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] Add helper for fnmatch over strv
No functional change intended.
---
src/analyze/analyze.c | 46 ---
src/libsystemd-network/network-internal.c | 53 +--
src/shared/strv.c | 10 ++
src/shared/strv.h | 9 ++
src/systemctl/systemctl.c | 51 -
5 files changed, 41 insertions(+), 128 deletions(-)
diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c
index 46a97eb8e7..672a0d7976 100644
--- a/src/analyze/analyze.c
+++ b/src/analyze/analyze.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#include "sd-bus.h"
#include "bus-util.h"
@@ -985,46 +984,15 @@ static int graph_one_property(sd_bus *bus, const UnitInfo
*u, const char* prop,
return r;
STRV_FOREACH(unit, units) {
-char **p;
-bool match_found;
-
-if (!strv_isempty(arg_dot_from_patterns)) {
-match_found = false;
-
-STRV_FOREACH(p, arg_dot_from_patterns)
-if (fnmatch(*p, u->id, 0) == 0) {
-match_found = true;
-break;
-}
-
-if (!match_found)
-continue;
-}
-
-if (!strv_isempty(arg_dot_to_patterns)) {
-match_found = false;
-
-STRV_FOREACH(p, arg_dot_to_patterns)
-if (fnmatch(*p, *unit, 0) == 0) {
-match_found = true;
-break;
-}
-
-if (!match_found)
-continue;
-}
+if (!strv_fnmatch_or_empty(u->id, arg_dot_from_patterns, 0))
+continue;
-if (!strv_isempty(patterns)) {
-match_found = false;
+if (!strv_fnmatch_or_empty(*unit, arg_dot_to_patterns, 0))
+continue;
-STRV_FOREACH(p, patterns)
-if (fnmatch(*p, u->id, 0) == 0 || fnmatch(*p,
*unit, 0) == 0) {
-match_found = true;
-break;
-}
-if (!match_found)
-continue;
-}
+if (!strv_fnmatch_or_empty(u->id, patterns, 0) &&
+!strv_fnmatch_or_empty(*unit, patterns, 0))
+continue;
printf("\t\"%s\"->\"%s\" [color=\"%s\"];\n", u->id, *unit,
color);
}
diff --git a/src/libsystemd-network/network-internal.c
b/src/libsystemd-network/network-internal.c
index 41f43d3389..5867aef662 100644
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include "strv.h"
#include "siphash24.h"
@@ -97,10 +96,6 @@ bool net_match_config(const struct ether_addr *match_mac,
const char *dev_driver,
const char *dev_type,
const char *dev_name) {
-char * const *match_path;
-char * const *match_driver;
-char * const *match_type;
-char * const *match_name;
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);
-STRV_FOREACH(match_path, match_paths)
-if (fnmatch(*match_path, dev_path, 0) == 0)
-return true;
+if (!strv_isempty(match_drivers))
+return strv_fnmatch(dev_driver, match_drivers, 0);
-return false;
-}
-
-if (!strv_isempty(match_drivers)) {
-if (!dev_driver)
-return false;
-
-STRV_FOREACH(match_driver, match_drivers)
-if (fnmatch(*match_driver, dev_driver, 0) == 0)
-return true;
-
-return false;
-}
-
-if (!strv_isempty(match_types)) {
-if (!dev_type)
-return false;
+if (!strv_isempty(match_types))
+return strv_fnmatch(dev_type, match_types, 0);
-ST
