On Mon, Oct 29, 2018 at 06:31:10PM +0100, Pablo Neira Ayuso wrote:
> @Phil, thinking here we could probably get rid of
> NFT_CTX_OUTPUT_NUMERIC_PROTO, since it is contained already in
> NFT_CTX_OUTPUT_NUMERIC_SYMBOL.
>
> There's no option for -p anymore, so we could simply things a bit
> before.
I think it's feasible to provide cmdline options for each bit '-n'
enables. Not only because this gives users control, it also simplifies
documentation a bit, like:
| -a - numeric a
| -b - numeric b
| -n - enable all numeric options from above
:)
[...]
> > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > index fb81edc0df07..82b4181de0cc 100644
> > --- a/include/nftables/libnftables.h
> > +++ b/include/nftables/libnftables.h
[...]
> > @@ -54,13 +47,12 @@ enum {
> > NFT_CTX_OUTPUT_GUID = (1 << 6),
> > NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
> > NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
> > + NFT_CTX_OUTPUT_NUMERIC_SYMBOL = (1 << 9),
> > };
+ #define NFT_CTX_OUTPUT_NUMERIC_ALL (NFT_CTX_OUTPUT_NUMERIC_PROTO |
+ NFT_CTX_OUTPUT_NUMERIC_PRIO |
+ NFT_CTX_OUTPUT_NUMERIC_SYMBOL)
[...]
> > diff --git a/src/main.c b/src/main.c
> > index 883261fc9d8b..952ce356490f 100644
> > --- a/src/main.c
> > +++ b/src/main.c
[...]
> > @@ -229,14 +226,10 @@ int main(int argc, char * const *argv)
> > }
> > break;
> > case OPT_NUMERIC:
> > - numeric = nft_ctx_output_get_numeric(nft);
> > - if (numeric == NFT_NUMERIC_ALL) {
> > - fprintf(stderr, "Too many numeric options "
> > - "used, max. %u\n",
> > - NFT_NUMERIC_ALL);
> > - exit(EXIT_FAILURE);
> > - }
> > - nft_ctx_output_set_numeric(nft, numeric + 1);
> > + numeric = true;
> > + output_flags |= (NFT_CTX_OUTPUT_NUMERIC_PROTO |
> > + NFT_CTX_OUTPUT_NUMERIC_PRIO |
> > + NFT_CTX_OUTPUT_NUMERIC_SYMBOL);
+ output_flags |= NFT_CTX_OUTPUT_NUMERIC_ALL;
> > @@ -298,6 +291,13 @@ int main(int argc, char * const *argv)
> > }
> > }
> >
> > + if (numeric &&
> > + (output_flags &
> > + (NFT_CTX_OUTPUT_REVERSEDNS |
> > + NFT_CTX_OUTPUT_SERVICE |
> > + NFT_CTX_OUTPUT_GUID)))
> > + fprintf(stderr, "cannot combine `-n' with `-N', `-S', '-u'\n");
> > +
Why not? We could allow people to call 'nft -n -N' to make all numeric
but enable reverse DNS (IIRC). All we need to do is document that '-n'
is an alias for '-p -q ...' and that later flags overwrite earlier ones
(as usual with getopt()-based tools). Would simplify code a bit, too.
> > diff --git a/src/monitor.c b/src/monitor.c
> > index b2267e1f63e4..0e735ed5b1aa 100644
> > --- a/src/monitor.c
> > +++ b/src/monitor.c
> > @@ -835,11 +835,9 @@ static int netlink_events_newgen_cb(const struct nlmsghdr *nlh, int type,
> > }
> > if (genid >= 0) {
> > nft_mon_print(monh, "# new generation %d", genid);
> > - if (pid >= 0) {
> > - nft_mon_print(monh, " by process %d", pid);
> > - if (!monh->ctx->nft->output.numeric)
> > - nft_mon_print(monh, " (%s)", name);
> > - }
> > + if (pid >= 0)
> > + nft_mon_print(monh, " by process %d (%s)", pid, name);
> > +
> > nft_mon_print(monh, "\n");
> > }
> >
Yes, I think that's OK. People may still do 's/ (.*$//' if they don't
want it there.
Cheers, Phil
