Re: [iptables PATCH v3 0/7] Improve xtables-restore performance

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Wed, Nov 06, 2019 at 10:24:52AM +0100, Pablo Neira Ayuso wrote:
[...]
> One thing: why do you need the conversion from \n to \0. The idea is
> to read once from the file and keep it in a buffer, then pass it to
> the original parsing function after this pre-parsing to calculate the
> cache.

Excellent question! It took me quite a while to figure out why it is
necessary to drop the trailing newlines when buffering input: In
add_param_to_argv() I couldn't find what my comment described, yet when
I removed the newline character dropping code some shell tests started
failing.

The real reason is this: When reading a table or chain definition line,
xtables_restore_parse_line() uses strtok() to eliminate trailing
whitespace or newline characters. This in turn mangles input buffer,
replacing the newline chars by nul chars.

The above turns into a problem when xtables_restore_parse() then updates
the pointer to the next string in buffer by calling:

| ptr += strlen(ptr) + 1;

With double nul chars, 'ptr' will point at the second one and that
matches the loop exit condition so we'll lose the remaining buffered
lines.

I'll fix the comment before pushing the commits out.

> Please, add this to the remaining patches of this series.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks, Phil