src: Always print range expressions numerically

PATCH In review Elise Lennion 2 participants 2 messages Jan 30, 2017 – Feb 1, 2017 netfilter

Share & tools

Elise Lennion

author

2017-01-30 14:12 UTC #0 lore raw

PATCH 1/1 src: Always print range expressions numerically

Because the rules are more legible this way. Also, the parser doesn't accept strings on ranges, so, printing ranges numerically better match the rules definition.

Fixes(Bug 1046 - mobility header with range gives illegible rule).

A new NUMERIC constant was defined to fill the previous role of NUMERIC_ALL, so the option -nnn doesn't affect symbolic constants.

Signed-off-by Elise Lennion <[email protected]>

---
 include/nftables.h |  1 +
 src/datatype.c     | 11 +++++++----
 src/expression.c   |  2 ++
 src/main.c         |  3 ++-
 src/meta.c         |  4 ++--
 5 files changed, 14 insertions(+), 7 deletions(-)

include/nftables.h +1 -0

index 6f54155..2730f65 100644
@@ -9,6 +9,7 @@ enum numeric_level {
 	NUMERIC_NONE,
 	NUMERIC_ADDR,
 	NUMERIC_PORT,
+	NUMERIC_PROTO_UGID,
 	NUMERIC_ALL,
 };

src/datatype.c +7 -4

index 1518606..f9981a6 100644
@@ -175,9 +175,12 @@ void symbolic_constant_print(const struct symbol_table *tbl,
 		return expr_basetype(expr)->print(expr);
 
 	if (quotes)
-		printf("\"%s\"", s->identifier);
-	else
-		printf("%s", s->identifier);
+		printf("\"");
+
+	numeric_output >= NUMERIC_ALL ? printf("%lu", val) : printf("%s", s->identifier);
+
+	if (quotes)
+		printf("\"");
 }
 
 static void switch_byteorder(void *data, unsigned int len)

@@ -530,7 +533,7 @@ static void inet_protocol_type_print(const struct expr *expr)
 {
 	struct protoent *p;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		p = getprotobynumber(mpz_get_uint8(expr->value));
 		if (p != NULL) {
 			printf("%s", p->p_name);

src/expression.c +2 -0

index 1567870..bdbbd45 100644
@@ -597,9 +597,11 @@ struct expr *relational_expr_alloc(const struct location *loc, enum ops op,
 
 static void range_expr_print(const struct expr *expr)
 {
+	numeric_output += NUMERIC_ALL;
 	expr_print(expr->left);
 	printf("-");
 	expr_print(expr->right);
+	numeric_output -= NUMERIC_ALL;
 }
 
 static void range_expr_clone(struct expr *new, const struct expr *expr)

src/main.c +2 -1

index 6ba752b..1b28836 100644
@@ -288,7 +288,8 @@ int main(int argc, char * const *argv)
 			include_paths[num_include_paths++] = optarg;
 			break;
 		case OPT_NUMERIC:
-			numeric_output++;
+			if (numeric_output + 1 < NUMERIC_ALL)
+				numeric_output++;
 			break;
 		case OPT_STATELESS:
 			stateless_output++;

src/meta.c +2 -2

index cb7c136..9e8a987 100644
@@ -226,7 +226,7 @@ static void uid_type_print(const struct expr *expr)
 {
 	struct passwd *pw;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		uint32_t uid = mpz_get_uint32(expr->value);
 
 		pw = getpwuid(uid);

@@ -278,7 +278,7 @@ static void gid_type_print(const struct expr *expr)
 {
 	struct group *gr;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		uint32_t gid = mpz_get_uint32(expr->value);
 
 		gr = getgrgid(gid);

Signature · 1 line

2.7.4

Pablo Neira Ayuso

maintainer

2017-02-01 17:58 UTC #1 lore raw

Quoted 23 lines · Elise Lennion

Because the rules are more legible this way. Also, the parser doesn't accept strings on ranges, so, printing ranges numerically better match the rules definition.
Fixes(Bug 1046 - mobility header with range gives illegible rule).
A new NUMERIC constant was defined to fill the previous role of NUMERIC_ALL, so the option -nnn doesn't affect symbolic constants.
Signed-off-by Elise Lennion <[email protected]>
---
 include/nftables.h |  1 +
 src/datatype.c     | 11 +++++++----
 src/expression.c   |  2 ++
 src/main.c         |  3 ++-
 src/meta.c         |  4 ++--
 5 files changed, 14 insertions(+), 7 deletions(-)
include/nftables.h +0 -0
index 6f54155..2730f65 100644

include/nftables.h enum numeric_level { · line 12

Quoted 4 lines

include/nftables.h +1 -0

@@ -9,6 +9,7 @@ enum numeric_level {
 	NUMERIC_NONE,
 	NUMERIC_ADDR,
 	NUMERIC_PORT,
+	NUMERIC_PROTO_UGID,

Do we need this necessarily?

Quoted 8 lines

include/nftables.h +0 -0
 	NUMERIC_ALL,
 };
 
src/datatype.c +0 -0
index 1518606..f9981a6 100644

src/datatype.c void symbolic_constant_print(const struct symbol_table *tbl, · line 180

Quoted 9 lines

src/datatype.c +3 -3

@@ -175,9 +175,12 @@ void symbolic_constant_print(const struct symbol_table *tbl,
 		return expr_basetype(expr)->print(expr);
 
 	if (quotes)
-		printf("\"%s\"", s->identifier);
-	else
-		printf("%s", s->identifier);
+		printf("\"");
+
+	numeric_output >= NUMERIC_ALL ? printf("%lu", val) : printf("%s", s->identifier);

Use if () here instead.

We usually leave this syntax to use it from return statements.

Quoted 36 lines

src/datatype.c +4 -1

+
+	if (quotes)
+		printf("\"");
 }
 
 static void switch_byteorder(void *data, unsigned int len)

@@ -530,7 +533,7 @@ static void inet_protocol_type_print(const struct expr *expr)
 {
 	struct protoent *p;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		p = getprotobynumber(mpz_get_uint8(expr->value));
 		if (p != NULL) {
 			printf("%s", p->p_name);

src/expression.c +2 -0

index 1567870..bdbbd45 100644
@@ -597,9 +597,11 @@ struct expr *relational_expr_alloc(const struct location *loc, enum ops op,
 
 static void range_expr_print(const struct expr *expr)
 {
+	numeric_output += NUMERIC_ALL;
 	expr_print(expr->left);
 	printf("-");
 	expr_print(expr->right);
+	numeric_output -= NUMERIC_ALL;
 }
 
 static void range_expr_clone(struct expr *new, const struct expr *expr)

src/main.c +0 -0

index 6ba752b..1b28836 100644

src/main.c int main(int argc, char * const *argv) · line 292

Quoted 6 lines

src/main.c +2 -1

@@ -288,7 +288,8 @@ int main(int argc, char * const *argv)
 			include_paths[num_include_paths++] = optarg;
 			break;
 		case OPT_NUMERIC:
-			numeric_output++;
+			if (numeric_output + 1 < NUMERIC_ALL)
+				numeric_output++;

This is catching a different problem, right? Makes sure we validate the number of -n. I'd suggest a separated patch to this fix.

I suggest we print an error here if the number of -n is exceeded.

Quoted 28 lines

src/main.c +0 -0

 			break;
 		case OPT_STATELESS:
 			stateless_output++;

src/meta.c +2 -2

index cb7c136..9e8a987 100644
@@ -226,7 +226,7 @@ static void uid_type_print(const struct expr *expr)
 {
 	struct passwd *pw;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		uint32_t uid = mpz_get_uint32(expr->value);
 
 		pw = getpwuid(uid);

@@ -278,7 +278,7 @@ static void gid_type_print(const struct expr *expr)
 {
 	struct group *gr;
 
-	if (numeric_output < NUMERIC_ALL) {
+	if (numeric_output < NUMERIC_PROTO_UGID) {
 		uint32_t gid = mpz_get_uint32(expr->value);
 
 		gr = getgrgid(gid);

Signature · 1 line

2.7.4