| Message ID | 1492704342-24042-6-git-send-email-pme.lebleu@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Inline… > On Apr 20, 2017, at 10:05 AM, Pierre Lebleu <pme.lebleu@gmail.com> wrote: > > It gives the ability to include scripts via procd > services and netifd interface firewall data. > > Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com> > --- > includes.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++----------- > includes.h | 9 ++++++--- > main.c | 2 +- > 3 files changed, 60 insertions(+), 15 deletions(-) > > diff --git a/includes.c b/includes.c > index 7ca164f..f373c9f 100644 > --- a/includes.c > +++ b/includes.c > @@ -31,15 +31,58 @@ const struct fw3_option fw3_include_opts[] = { > }; > > > +static struct fw3_include * > +fw3_alloc_include(struct fw3_state *state) > +{ > + struct fw3_include *include; > + > + include = calloc(1, sizeof(*include)); > + if (!include) > + return NULL; > + > + include->enabled = true; > + > + list_add_tail(&include->list, &state->includes); > + > + return include; > +} > + > void > -fw3_load_includes(struct fw3_state *state, struct uci_package *p) > +fw3_load_includes(struct fw3_state *state, struct uci_package *p, > + struct blob_attr *a) > { > struct uci_section *s; > struct uci_element *e; > - struct fw3_include *include; > + struct fw3_include *include, *n; > + struct blob_attr *entry, *opt; > + unsigned rem, orem; > > INIT_LIST_HEAD(&state->includes); > > + blob_for_each_attr(entry, a, rem) > + { > + const char *type = NULL; > + const char *name = "ubus include"; > + blobmsg_for_each_attr(opt, entry, orem) > + if (!strcmp(blobmsg_name(opt), "type")) > + type = blobmsg_get_string(opt); > + else if (!strcmp(blobmsg_name(opt), "name")) > + name = blobmsg_get_string(opt); > + > + if (!type || (strcmp(type, "script") && strcmp(type, "restore"))) > + continue; > + > + if (!(include = fw3_alloc_include(state))) Same issue: assignments in conditionals are hard to step through with a source-level debugger. > + continue; > + > + if (!fw3_parse_blob_options(include, fw3_include_opts, entry, name)) > + { > + warn("%s skipped due to invalid options\n", name); Don’t most warnings NOT terminate with a newline? Why is this one different? > + fw3_free_include(include); > + continue; > + } > + } > + > uci_foreach_element(&p->sections, e) > { > s = uci_to_section(e); > @@ -47,12 +90,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) > if (strcmp(s->type, "include")) > continue; > > - include = calloc(1, sizeof(*include)); > - if (!include) > + if (!(include = fw3_alloc_include(state))) > continue; > > include->name = e->name; > - include->enabled = true; > > if (!fw3_parse_options(include, fw3_include_opts, s)) > { > @@ -60,7 +101,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) > fw3_free_include(include); > continue; > } > + } > > + list_for_each_entry_safe(include, n, &state->includes, list) > + { > if (!include->enabled) > { > fw3_free_include(include); > @@ -69,17 +113,15 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) > > if (!include->path) > { > - warn_elem(e, "must specify a path"); > + warn("%s must specify a path", include->name); > fw3_free_include(include); > continue; > } > > if (include->type == FW3_INC_TYPE_RESTORE && !include->family) > - warn_elem(e, "does not specify a family, include will get loaded " > - "with both iptables-restore and ip6tables-restore!"); > - > - list_add_tail(&include->list, &state->includes); > - continue; > + warn("%s does not specify a family, include will get loaded " > + "with both iptables-restore and ip6tables-restore!", > + include->name); > } > } > > diff --git a/includes.h b/includes.h > index 070cb3a..3a0af1b 100644 > --- a/includes.h > +++ b/includes.h > @@ -24,14 +24,17 @@ > > extern const struct fw3_option fw3_include_opts[]; > > -void fw3_load_includes(struct fw3_state *state, struct uci_package *p); > +void fw3_load_includes(struct fw3_state *state, struct uci_package *p, struct blob_attr *a); > > void fw3_print_includes(struct fw3_state *state, enum fw3_family family, > bool reload); > > void fw3_run_includes(struct fw3_state *state, bool reload); > > -#define fw3_free_include(include) \ > - fw3_free_object(include, fw3_include_opts) > +static inline void fw3_free_include(struct fw3_include *include) > +{ > + list_del(&include->list); > + fw3_free_object(include, fw3_include_opts); > +} > > #endif > diff --git a/main.c b/main.c > index 6e275ef..c4b8228 100644 > --- a/main.c > +++ b/main.c > @@ -107,7 +107,7 @@ build_state(bool runtime) > fw3_load_redirects(state, p, b.head); > fw3_load_snats(state, p, b.head); > fw3_load_forwards(state, p, b.head); > - fw3_load_includes(state, p); > + fw3_load_includes(state, p, b.head); > > return true; > } > -- > 1.7.9.5 > > > _______________________________________________ > Lede-dev mailing list > Lede-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/lede-dev
2017-04-29 3:13 GMT+02:00 Philip Prindeville <philipp_subx@redfish-solutions.com>: > Inline… > > >> On Apr 20, 2017, at 10:05 AM, Pierre Lebleu <pme.lebleu@gmail.com> wrote: >> >> It gives the ability to include scripts via procd >> services and netifd interface firewall data. >> >> Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com> >> --- >> includes.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++----------- >> includes.h | 9 ++++++--- >> main.c | 2 +- >> 3 files changed, 60 insertions(+), 15 deletions(-) >> >> diff --git a/includes.c b/includes.c >> index 7ca164f..f373c9f 100644 >> --- a/includes.c >> +++ b/includes.c >> @@ -31,15 +31,58 @@ const struct fw3_option fw3_include_opts[] = { >> }; >> >> >> +static struct fw3_include * >> +fw3_alloc_include(struct fw3_state *state) >> +{ >> + struct fw3_include *include; >> + >> + include = calloc(1, sizeof(*include)); >> + if (!include) >> + return NULL; >> + >> + include->enabled = true; >> + >> + list_add_tail(&include->list, &state->includes); >> + >> + return include; >> +} >> + >> void >> -fw3_load_includes(struct fw3_state *state, struct uci_package *p) >> +fw3_load_includes(struct fw3_state *state, struct uci_package *p, >> + struct blob_attr *a) >> { >> struct uci_section *s; >> struct uci_element *e; >> - struct fw3_include *include; >> + struct fw3_include *include, *n; >> + struct blob_attr *entry, *opt; >> + unsigned rem, orem; >> >> INIT_LIST_HEAD(&state->includes); >> >> + blob_for_each_attr(entry, a, rem) >> + { >> + const char *type = NULL; >> + const char *name = "ubus include"; >> + blobmsg_for_each_attr(opt, entry, orem) >> + if (!strcmp(blobmsg_name(opt), "type")) >> + type = blobmsg_get_string(opt); >> + else if (!strcmp(blobmsg_name(opt), "name")) >> + name = blobmsg_get_string(opt); >> + >> + if (!type || (strcmp(type, "script") && strcmp(type, "restore"))) >> + continue; >> + >> + if (!(include = fw3_alloc_include(state))) > > > Same issue: assignments in conditionals are hard to step through with a source-level debugger. Fixed as suggested. > > >> + continue; >> + >> + if (!fw3_parse_blob_options(include, fw3_include_opts, entry, name)) >> + { >> + warn("%s skipped due to invalid options\n", name); > > > Don’t most warnings NOT terminate with a newline? Why is this one different? It was a copy/paste from "rules.c" and indeed, it is a mistake. > > >> + fw3_free_include(include); >> + continue; >> + } >> + } >> + >> uci_foreach_element(&p->sections, e) >> { >> s = uci_to_section(e); >> @@ -47,12 +90,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) >> if (strcmp(s->type, "include")) >> continue; >> >> - include = calloc(1, sizeof(*include)); >> - if (!include) >> + if (!(include = fw3_alloc_include(state))) >> continue; >> >> include->name = e->name; >> - include->enabled = true; >> >> if (!fw3_parse_options(include, fw3_include_opts, s)) >> { >> @@ -60,7 +101,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) >> fw3_free_include(include); >> continue; >> } >> + } >> >> + list_for_each_entry_safe(include, n, &state->includes, list) >> + { >> if (!include->enabled) >> { >> fw3_free_include(include); >> @@ -69,17 +113,15 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) >> >> if (!include->path) >> { >> - warn_elem(e, "must specify a path"); >> + warn("%s must specify a path", include->name); >> fw3_free_include(include); >> continue; >> } >> >> if (include->type == FW3_INC_TYPE_RESTORE && !include->family) >> - warn_elem(e, "does not specify a family, include will get loaded " >> - "with both iptables-restore and ip6tables-restore!"); >> - >> - list_add_tail(&include->list, &state->includes); >> - continue; >> + warn("%s does not specify a family, include will get loaded " >> + "with both iptables-restore and ip6tables-restore!", >> + include->name); >> } >> } >> >> diff --git a/includes.h b/includes.h >> index 070cb3a..3a0af1b 100644 >> --- a/includes.h >> +++ b/includes.h >> @@ -24,14 +24,17 @@ >> >> extern const struct fw3_option fw3_include_opts[]; >> >> -void fw3_load_includes(struct fw3_state *state, struct uci_package *p); >> +void fw3_load_includes(struct fw3_state *state, struct uci_package *p, struct blob_attr *a); >> >> void fw3_print_includes(struct fw3_state *state, enum fw3_family family, >> bool reload); >> >> void fw3_run_includes(struct fw3_state *state, bool reload); >> >> -#define fw3_free_include(include) \ >> - fw3_free_object(include, fw3_include_opts) >> +static inline void fw3_free_include(struct fw3_include *include) >> +{ >> + list_del(&include->list); >> + fw3_free_object(include, fw3_include_opts); >> +} >> >> #endif >> diff --git a/main.c b/main.c >> index 6e275ef..c4b8228 100644 >> --- a/main.c >> +++ b/main.c >> @@ -107,7 +107,7 @@ build_state(bool runtime) >> fw3_load_redirects(state, p, b.head); >> fw3_load_snats(state, p, b.head); >> fw3_load_forwards(state, p, b.head); >> - fw3_load_includes(state, p); >> + fw3_load_includes(state, p, b.head); >> >> return true; >> } >> -- >> 1.7.9.5 >> >> >> _______________________________________________ >> Lede-dev mailing list >> Lede-dev@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/lede-dev >
diff --git a/includes.c b/includes.c index 7ca164f..f373c9f 100644 --- a/includes.c +++ b/includes.c @@ -31,15 +31,58 @@ const struct fw3_option fw3_include_opts[] = { }; +static struct fw3_include * +fw3_alloc_include(struct fw3_state *state) +{ + struct fw3_include *include; + + include = calloc(1, sizeof(*include)); + if (!include) + return NULL; + + include->enabled = true; + + list_add_tail(&include->list, &state->includes); + + return include; +} + void -fw3_load_includes(struct fw3_state *state, struct uci_package *p) +fw3_load_includes(struct fw3_state *state, struct uci_package *p, + struct blob_attr *a) { struct uci_section *s; struct uci_element *e; - struct fw3_include *include; + struct fw3_include *include, *n; + struct blob_attr *entry, *opt; + unsigned rem, orem; INIT_LIST_HEAD(&state->includes); + blob_for_each_attr(entry, a, rem) + { + const char *type = NULL; + const char *name = "ubus include"; + blobmsg_for_each_attr(opt, entry, orem) + if (!strcmp(blobmsg_name(opt), "type")) + type = blobmsg_get_string(opt); + else if (!strcmp(blobmsg_name(opt), "name")) + name = blobmsg_get_string(opt); + + if (!type || (strcmp(type, "script") && strcmp(type, "restore"))) + continue; + + if (!(include = fw3_alloc_include(state))) + continue; + + if (!fw3_parse_blob_options(include, fw3_include_opts, entry, name)) + { + warn("%s skipped due to invalid options\n", name); + fw3_free_include(include); + continue; + } + } + uci_foreach_element(&p->sections, e) { s = uci_to_section(e); @@ -47,12 +90,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) if (strcmp(s->type, "include")) continue; - include = calloc(1, sizeof(*include)); - if (!include) + if (!(include = fw3_alloc_include(state))) continue; include->name = e->name; - include->enabled = true; if (!fw3_parse_options(include, fw3_include_opts, s)) { @@ -60,7 +101,10 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) fw3_free_include(include); continue; } + } + list_for_each_entry_safe(include, n, &state->includes, list) + { if (!include->enabled) { fw3_free_include(include); @@ -69,17 +113,15 @@ fw3_load_includes(struct fw3_state *state, struct uci_package *p) if (!include->path) { - warn_elem(e, "must specify a path"); + warn("%s must specify a path", include->name); fw3_free_include(include); continue; } if (include->type == FW3_INC_TYPE_RESTORE && !include->family) - warn_elem(e, "does not specify a family, include will get loaded " - "with both iptables-restore and ip6tables-restore!"); - - list_add_tail(&include->list, &state->includes); - continue; + warn("%s does not specify a family, include will get loaded " + "with both iptables-restore and ip6tables-restore!", + include->name); } } diff --git a/includes.h b/includes.h index 070cb3a..3a0af1b 100644 --- a/includes.h +++ b/includes.h @@ -24,14 +24,17 @@ extern const struct fw3_option fw3_include_opts[]; -void fw3_load_includes(struct fw3_state *state, struct uci_package *p); +void fw3_load_includes(struct fw3_state *state, struct uci_package *p, struct blob_attr *a); void fw3_print_includes(struct fw3_state *state, enum fw3_family family, bool reload); void fw3_run_includes(struct fw3_state *state, bool reload); -#define fw3_free_include(include) \ - fw3_free_object(include, fw3_include_opts) +static inline void fw3_free_include(struct fw3_include *include) +{ + list_del(&include->list); + fw3_free_object(include, fw3_include_opts); +} #endif diff --git a/main.c b/main.c index 6e275ef..c4b8228 100644 --- a/main.c +++ b/main.c @@ -107,7 +107,7 @@ build_state(bool runtime) fw3_load_redirects(state, p, b.head); fw3_load_snats(state, p, b.head); fw3_load_forwards(state, p, b.head); - fw3_load_includes(state, p); + fw3_load_includes(state, p, b.head); return true; }
It gives the ability to include scripts via procd services and netifd interface firewall data. Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com> --- includes.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++----------- includes.h | 9 ++++++--- main.c | 2 +- 3 files changed, 60 insertions(+), 15 deletions(-)