syntax: replace '=>' with '=:'

RFC PATCH In review Arturo Borrero Gonzalez 3 participants 7 messages Jan 12, 2014 – Jan 13, 2014 netfilter

Share & tools

Arturo Borrero Gonzalez

author

2014-01-12 19:41 UTC #0 lore raw

RFC PATCH 1/1 syntax: replace '=>' with '=:'

Almost all shell uses the '>' character as a key for redirecting stdout/stderr to a file.
So, using it in the syntax means that the administrator is forced to scape the character, or look for other workaround.

With this patch, '=>' is replaced with '=:', thus avoiding such situation.

Signed-off-by Arturo Borrero Gonzalez <[email protected]>

---
 src/expression.c |    2 +-
 src/parser.y     |    7 ++++---
 src/rule.c       |    2 +-
 src/scanner.l    |    1 +
 4 files changed, 7 insertions(+), 5 deletions(-)

src/expression.c +1 -1

index 71154cc..b9df9ac 100644
@@ -642,7 +642,7 @@ struct expr *set_expr_alloc(const struct location *loc)
 static void mapping_expr_print(const struct expr *expr)
 {
 	expr_print(expr->left);
-	printf(" => ");
+	printf(" =: ");
 	expr_print(expr->right);
 }

src/parser.y +4 -3

index 26e71e3..577aba1 100644
@@ -150,6 +150,7 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token DASH			"-"
 %token AT			"@"
 %token ARROW			"=>"
+%token MAP_SIGN			"=:"
 %token VMAP			"vmap"
 
 %token INCLUDE			"include"

@@ -751,7 +752,7 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 			|	map_block	common_block
 			|	map_block	stmt_seperator
 			|	map_block	TYPE
-						identifier	ARROW	identifier
+						identifier	MAP_SIGN	identifier
 						stmt_seperator
 			{
 				$1->keytype = datatype_lookup_byname($3);

@@ -1243,11 +1244,11 @@ set_list_member_expr	:	opt_newline	expr	opt_newline
 			{
 				$$ = $2;
 			}
-			|	opt_newline	map_lhs_expr	ARROW	concat_expr	opt_newline
+			|	opt_newline	map_lhs_expr	MAP_SIGN	concat_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}
-			|	opt_newline	map_lhs_expr	ARROW	verdict_expr	opt_newline
+			|	opt_newline	map_lhs_expr	MAP_SIGN	verdict_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}

src/rule.c +1 -1

index ec8b6a4..b593624 100644
@@ -96,7 +96,7 @@ void set_print(const struct set *set)
 
 	printf("\t\ttype %s", set->keytype->name);
 	if (set->flags & SET_F_MAP)
-		printf(" => %s", set->datatype->name);
+		printf(" =: %s", set->datatype->name);
 	printf("\n");
 
 	if (set->flags & SET_F_ANONYMOUS)

src/scanner.l +1 -0

index cee6aa6..14470cf 100644
@@ -210,6 +210,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "$"			{ return '$'; }
 "="			{ return '='; }
 "=>"			{ return ARROW; }
+"=:"			{ return MAP_SIGN; }
 "vmap"			{ return VMAP; }
 
 "include"		{ return INCLUDE; }

Patrick McHardy

2014-01-12 20:17 UTC #1 lore raw

Quoted 6 lines · Arturo Borrero Gonzalez

Almost all shell uses the '>' character as a key for redirecting stdout/stderr to a file.
So, using it in the syntax means that the administrator is forced to scape the character, or look for other workaround.
With this patch, '=>' is replaced with '=:', thus avoiding such situation.

I'm not opposed to this, but I like (despite the shell problematic) the => syntax better, so I'd suggest to just add an alternative syntax.

As further simplification, why not simply use ':'?

Arturo Borrero Gonzalez

author

2014-01-12 20:28 UTC #2 lore raw

Quoted 6 lines · Patrick McHardy

Quoted 5 lines · Arturo Borrero Gonzalez
Almost all shell uses the '>' character as a key for redirecting stdout/stderr to a file.
So, using it in the syntax means that the administrator is forced to scape the character, or look for other workaround.

Quoted 6 lines

Quoted 1 line
With this patch, '=>' is replaced with '=:', thus avoiding such situation.
I'm not opposed to this, but I like (despite the shell problematic) the => syntax better, so I'd suggest to just add an alternative syntax.
As further simplification, why not simply use ':'?

Well, I also like using just ':'

But maybe we clash with IPv6 addresses in some cases:

nft add rule ip6 filter input ip6 saddr vmap { ::1 : accept , ::2 : drop } nft add rule ip6 filter input ip6 saddr vmap { ::1:accept , ::2:drop }

nft add rule ip6 filter input meta dnat set tcp dport map { 80 : ::1, 8888 : ::2 }
nft add rule ip6 filter input meta dnat set tcp dport map { 80:::1, 8888:::2 }

what do you think?

Signature · 5 lines

Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy

2014-01-12 21:25 UTC #3 lore raw

Quoted 18 lines · Arturo Borrero Gonzalez

Quoted 6 lines · Patrick McHardy
Quoted 5 lines · Arturo Borrero Gonzalez
Almost all shell uses the '>' character as a key for redirecting stdout/stderr to a file.
So, using it in the syntax means that the administrator is forced to scape the character, or look for other workaround.
Quoted 6 lines
Quoted 1 line
With this patch, '=>' is replaced with '=:', thus avoiding such situation.
I'm not opposed to this, but I like (despite the shell problematic) the => syntax better, so I'd suggest to just add an alternative syntax.
As further simplification, why not simply use ':'?
Well, I also like using just ':'
But maybe we clash with IPv6 addresses in some cases:

Quoted 8 lines

nft add rule ip6 filter input ip6 saddr vmap { ::1 : accept , ::2 : drop } nft add rule ip6 filter input ip6 saddr vmap { ::1:accept , ::2:drop }
nft add rule ip6 filter input meta dnat set tcp dport map { 80 : ::1, 8888 : ::2 }
nft add rule ip6 filter input meta dnat set tcp dport map { 80:::1, 8888:::2 }
what do you think?

IPv6 addresses are recognized by the parser, so it should be fine. I just compile tested this patch, at least bison doesn't report any errors.

src/netlink.c +1 -1

index 59bd8e4..0f8275c 100644
@@ -332,7 +332,7 @@ int netlink_add_rule_batch(struct netlink_ctx *ctx,
 					     ctx->seqnum);
 		if (err < 0)
 			netlink_io_error(ctx, &rule->location,
-					 "Could not add rule to batch: %s",
+					 "Could not add rule: %s",
 					 strerror(errno));
 	}
 	nft_rule_free(nlr);

src/parser.y +3 -4

index 7c18875..f91746a 100644
@@ -149,7 +149,6 @@ static void location_update(struct location *loc, struct location *rhs, int n)
 %token ASTERISK			"*"
 %token DASH			"-"
 %token AT			"@"
-%token ARROW			"=>"
 %token VMAP			"vmap"
 
 %token INCLUDE			"include"

@@ -764,7 +763,7 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 			|	map_block	common_block
 			|	map_block	stmt_seperator
 			|	map_block	TYPE
-						identifier	ARROW	identifier
+						identifier	COLON	identifier
 						stmt_seperator
 			{
 				$1->keytype = datatype_lookup_byname($3);

@@ -1309,11 +1308,11 @@ set_list_member_expr	:	opt_newline	expr	opt_newline
 			{
 				$$ = $2;
 			}
-			|	opt_newline	map_lhs_expr	ARROW	concat_expr	opt_newline
+			|	opt_newline	map_lhs_expr	COLON	concat_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}
-			|	opt_newline	map_lhs_expr	ARROW	verdict_expr	opt_newline
+			|	opt_newline	map_lhs_expr	COLON	verdict_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}

src/scanner.l +0 -1

index 0b8abac..9fa5471 100644
@@ -209,7 +209,6 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "@"			{ return AT; }
 "$"			{ return '$'; }
 "="			{ return '='; }
-"=>"			{ return ARROW; }
 "vmap"			{ return VMAP; }
 
 "include"		{ return INCLUDE; }

Arturo Borrero Gonzalez

author

2014-01-12 21:47 UTC #4 lore raw

Quoted 4 lines · Patrick McHardy

IPv6 addresses are recognized by the parser, so it should be fine. I just compile tested this patch, at least bison doesn't report any errors.

'=:' works also as a visual separator.

If using ':', in the case of IPv6, we can end with:
2a00:9ac0:c1ca:27::150:123

Bison may detect it wisely, but I don't know where the address start/ends, don't you?

Or we can force/require a space in the syntax:
2a00:9ac0:c1ca:27::150 : 123

If we decide to use ':', this last case is better, IMHO.

If you don't like '=:', what about:
'--'
'-:'
'---'
'--:'
or
':--'

Signature · 5 lines

Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patrick McHardy

2014-01-12 21:55 UTC #5 lore raw

Quoted 11 lines · Arturo Borrero Gonzalez

Quoted 4 lines · Patrick McHardy
IPv6 addresses are recognized by the parser, so it should be fine. I just compile tested this patch, at least bison doesn't report any errors.
'=:' works also as a visual separator.
If using ':', in the case of IPv6, we can end with:
2a00:9ac0:c1ca:27::150:123

Quoted 7 lines

Bison may detect it wisely, but I don't know where the address start/ends, don't you?
Or we can force/require a space in the syntax:
2a00:9ac0:c1ca:27::150 : 123
If we decide to use ':', this last case is better, IMHO.

We don't need to enforce this IMO. Any reasonable user will most likely add the space himself. If not, no problem, as long as there is no ambiguity.

Quoted 7 lines

If you don't like '=:', what about:
'--'
'-:'
'---'
'--:'
or
':--'

Neither of. Too long, and for a mapping ':' or '=>' seem a reasonable choice. ':=' is more like an assignment, which doesn't fit too well.

I'd say go for ':', if the user doesn't insert spaces and can't read his own rules anymore, his fault.

Pablo Neira Ayuso

maintainer

2014-01-13 10:44 UTC #6 lore raw

Quoted 8 lines · Patrick McHardy

Quoted 7 lines
If you don't like '=:', what about:
'--'
'-:'
'---'
'--:'
or
':--'

Quoted 5 lines

Neither of. Too long, and for a mapping ':' or '=>' seem a reasonable choice. ':=' is more like an assignment, which doesn't fit too well.
I'd say go for ':', if the user doesn't insert spaces and can't read his own rules anymore, his fault.

I like ':' is used in python dictinaries too. Erlang uses ',' as separator and ruby was using the '=>' that causes some troubles with bash if not escaped. I think there is not chance for ambiguity, but I'm going to make more tests and get back to you.