diff mbox

[1/2,nft,RFC] expression: default to print binary operations using nominal representation

Message ID 1389784167-10198-1-git-send-email-pablo@netfilter.org
State Not Applicable
Headers show

Commit Message

Pablo Neira Ayuso Jan. 15, 2014, 11:09 a.m. UTC
Since b59e65c ("scanner: add aliases to symbols for easier
interaction with most shells") we have nominal aliases, default to
these for the output representation.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/expression.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Patrick McHardy Jan. 15, 2014, 11:18 a.m. UTC | #1
On Wed, Jan 15, 2014 at 12:09:26PM +0100, Pablo Neira Ayuso wrote:
> Since b59e65c ("scanner: add aliases to symbols for easier
> interaction with most shells") we have nominal aliases, default to
> these for the output representation.

I was thinking about this before, but didn't mention it since you
hadn't included it in your first patch. I'd prefer to stick to the
IMO more readable existing form. Since we don't support deleting
rules by rule specification, there should be no reason to copy and
paste the output to the command line again.

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/expression.c |   20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/expression.c b/src/expression.c
> index 71154cc..6da5c10 100644
> --- a/src/expression.c
> +++ b/src/expression.c
> @@ -351,17 +351,17 @@ const char *expr_op_symbols[] = {
>  	[OP_INVALID]	= "invalid",
>  	[OP_HTON]	= "hton",
>  	[OP_NTOH]	= "ntoh",
> -	[OP_AND]	= "&",
> -	[OP_OR]		= "|",
> -	[OP_XOR]	= "^",
> -	[OP_LSHIFT]	= "<<",
> -	[OP_RSHIFT]	= ">>",
> +	[OP_AND]	= "and",
> +	[OP_OR]		= "or",
> +	[OP_XOR]	= "xor",
> +	[OP_LSHIFT]	= "lshift",
> +	[OP_RSHIFT]	= "rshift",
>  	[OP_EQ]		= NULL,
> -	[OP_NEQ]	= "!=",
> -	[OP_LT]		= "<",
> -	[OP_GT]		= ">",
> -	[OP_LTE]	= "<=",
> -	[OP_GTE]	= ">=",
> +	[OP_NEQ]	= "ne",
> +	[OP_LT]		= "lt",
> +	[OP_GT]		= "gt",
> +	[OP_LTE]	= "le",
> +	[OP_GTE]	= "ge",
>  	[OP_RANGE]	= "within range",
>  	[OP_LOOKUP]	= NULL,
>  };
> -- 
> 1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arturo Borrero Jan. 15, 2014, 11:32 a.m. UTC | #2
On 15 January 2014 12:18, Patrick McHardy <kaber@trash.net> wrote:
> On Wed, Jan 15, 2014 at 12:09:26PM +0100, Pablo Neira Ayuso wrote:
>> Since b59e65c ("scanner: add aliases to symbols for easier
>> interaction with most shells") we have nominal aliases, default to
>> these for the output representation.
>
> I was thinking about this before, but didn't mention it since you
> hadn't included it in your first patch. I'd prefer to stick to the
> IMO more readable existing form. Since we don't support deleting
> rules by rule specification, there should be no reason to copy and
> paste the output to the command line again.

What about a file to be loaded with `nft -f'?
In fact, I think most of users will end with a config file to load
with `nft -f'.

It may be annoying getting the output different to what you wrote.

I think this patch means that the symbol-free syntax is the default,
and I personally like that.
Patrick McHardy Jan. 15, 2014, 11:36 a.m. UTC | #3
On Wed, Jan 15, 2014 at 12:32:36PM +0100, Arturo Borrero Gonzalez wrote:
> On 15 January 2014 12:18, Patrick McHardy <kaber@trash.net> wrote:
> > On Wed, Jan 15, 2014 at 12:09:26PM +0100, Pablo Neira Ayuso wrote:
> >> Since b59e65c ("scanner: add aliases to symbols for easier
> >> interaction with most shells") we have nominal aliases, default to
> >> these for the output representation.
> >
> > I was thinking about this before, but didn't mention it since you
> > hadn't included it in your first patch. I'd prefer to stick to the
> > IMO more readable existing form. Since we don't support deleting
> > rules by rule specification, there should be no reason to copy and
> > paste the output to the command line again.
> 
> What about a file to be loaded with `nft -f'?
> In fact, I think most of users will end with a config file to load
> with `nft -f'.

Indeed, and those users don't need the long syntax since the shell is
not involved.

> It may be annoying getting the output different to what you wrote.

That may happen for other reasons anyways, especially with binops we
optimize away redundant expressions and calculate constants in userspace.

> I think this patch means that the symbol-free syntax is the default,
> and I personally like that.

I don't, its longer and harder to read. It has its justification for
parsing on the command line, but as you say, most users won't use it
that way.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/expression.c b/src/expression.c
index 71154cc..6da5c10 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -351,17 +351,17 @@  const char *expr_op_symbols[] = {
 	[OP_INVALID]	= "invalid",
 	[OP_HTON]	= "hton",
 	[OP_NTOH]	= "ntoh",
-	[OP_AND]	= "&",
-	[OP_OR]		= "|",
-	[OP_XOR]	= "^",
-	[OP_LSHIFT]	= "<<",
-	[OP_RSHIFT]	= ">>",
+	[OP_AND]	= "and",
+	[OP_OR]		= "or",
+	[OP_XOR]	= "xor",
+	[OP_LSHIFT]	= "lshift",
+	[OP_RSHIFT]	= "rshift",
 	[OP_EQ]		= NULL,
-	[OP_NEQ]	= "!=",
-	[OP_LT]		= "<",
-	[OP_GT]		= ">",
-	[OP_LTE]	= "<=",
-	[OP_GTE]	= ">=",
+	[OP_NEQ]	= "ne",
+	[OP_LT]		= "lt",
+	[OP_GT]		= "gt",
+	[OP_LTE]	= "le",
+	[OP_GTE]	= "ge",
 	[OP_RANGE]	= "within range",
 	[OP_LOOKUP]	= NULL,
 };