On Wed, Jan 12, 2022 at 12:42:19PM +0100, Pablo Neira Ayuso wrote:
> On Tue, Jan 11, 2022 at 04:04:18PM +0100, Phil Sutter wrote:
> > Patch 1 removes remains of an unused (and otherwise dropped) feature,
> > yet the change is necessary for the following ones. Patches 2-6 prepare
> > for patch 7 which moves do_parse() to xshared.c. Patches 8 and 9 prepare
> > for use of do_parse() from legacy code, Patches 10 and 11 finally drop
> > legacy ip(6)tables' rule parsing code.
>
> Just two nitpicks in case you would like to apply them before pushing
> out.
>
> - Patch #6
>
> diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
> index b211a30937db3..ba696c6a6a123 100644
> --- a/iptables/nft-arp.c
> +++ b/iptables/nft-arp.c
> @@ -802,7 +802,7 @@ struct nft_family_ops nft_family_ops_arp = {
> .print_rule = nft_arp_print_rule,
> .save_rule = nft_arp_save_rule,
> .save_chain = nft_arp_save_chain,
> - .post_parse = nft_arp_post_parse,
> + .cmd_parse.post_parse = nft_arp_post_parse,
> .rule_to_cs = nft_rule_to_iptables_command_state,
> .init_cs = nft_arp_init_cs,
> .clear_cs = nft_clear_iptables_command_state,
>
> I would use C99:
>
> .cmd_parse = {
> .post_parse = nft_arp_post_parse,
> },
>
> for future extensibility, but maybe it is too far fetched.
Fine with me!
> - Patch #10, instead of:
>
> + case CMD_NONE:
> + /* do_parse ignored the line (eg: -4 with ip6tables-restore) */
> + break;
>
> this:
>
> + case CMD_NONE:
> + /* do_parse ignored the line (eg: -4 with ip6tables-restore) */
> + break;
Oh yes, of course.
Thanks, Phil
