On Mon, Jan 10, 2022 at 10:29:37PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Dec 24, 2021 at 06:17:49PM +0100, Phil Sutter wrote:
> [...]
> > diff --git a/iptables/xshared.h b/iptables/xshared.h
> > index dde94b7335f6a..1954168f64058 100644
> > --- a/iptables/xshared.h
> > +++ b/iptables/xshared.h
> [...]
> > struct xt_cmd_parse {
> > unsigned int command;
> > unsigned int rulenum;
> > @@ -272,6 +305,11 @@ struct xt_cmd_parse {
> > bool restore;
> > int verbose;
> > bool xlate;
>
> Probably wrap these two common functions between legacy and nft in a
> structure? Something like: struct nft_parse_ops...
struct xt_parse_ops...
> > + void (*proto_parse)(struct iptables_command_state *cs,
> > + struct xtables_args *args);
> > + void (*post_parse)(int command,
> > + struct iptables_command_state *cs,
> > + struct xtables_args *args);
> > };
> >
> > #endif /* IPTABLES_XSHARED_H */
> > diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
> > index 9d312b244657e..b0b27695cbb8c 100644
> > --- a/iptables/xtables-translate.c
> > +++ b/iptables/xtables-translate.c
> > @@ -252,6 +252,8 @@ static int do_command_xlate(struct nft_handle *h, int argc, char *argv[],
> > .table = *table,
> > .restore = restore,
> > .xlate = true,
> > + .proto_parse = h->ops->proto_parse,
> > + .post_parse = h->ops->post_parse,
>
> so you could just do:
>
> .parse = h->ops->parse,
>
> and if you need to extend this structure in the future for whatever
> revolutionary reason, you will need to update this part of the code to
^....
you will *not* need
> do:
>
> .another_parse = h->ops->another_parse,
>
> Apart from this, anything else LGTM.
