* [PATCH nft] src: Always print range expressions numerically
@ 2017-01-30 14:05 Elise Lennion
2017-02-01 17:58 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Elise Lennion @ 2017-01-30 14:05 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
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 <elise.lennion@gmail.com>
---
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(-)
diff --git a/include/nftables.h b/include/nftables.h
index 6f54155..2730f65 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -9,6 +9,7 @@ enum numeric_level {
NUMERIC_NONE,
NUMERIC_ADDR,
NUMERIC_PORT,
+ NUMERIC_PROTO_UGID,
NUMERIC_ALL,
};
diff --git a/src/datatype.c b/src/datatype.c
index 1518606..f9981a6 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -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);
diff --git a/src/expression.c b/src/expression.c
index 1567870..bdbbd45 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -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)
diff --git a/src/main.c b/src/main.c
index 6ba752b..1b28836 100644
--- a/src/main.c
+++ b/src/main.c
@@ -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++;
diff --git a/src/meta.c b/src/meta.c
index cb7c136..9e8a987 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -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);
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH nft] src: Always print range expressions numerically
2017-01-30 14:05 [PATCH nft] src: Always print range expressions numerically Elise Lennion
@ 2017-02-01 17:58 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2017-02-01 17:58 UTC (permalink / raw)
To: Elise Lennion; +Cc: netfilter-devel
On Mon, Jan 30, 2017 at 12:05:20PM -0200, Elise Lennion wrote:
> 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 <elise.lennion@gmail.com>
> ---
> 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(-)
>
> diff --git a/include/nftables.h b/include/nftables.h
> index 6f54155..2730f65 100644
> --- a/include/nftables.h
> +++ b/include/nftables.h
> @@ -9,6 +9,7 @@ enum numeric_level {
> NUMERIC_NONE,
> NUMERIC_ADDR,
> NUMERIC_PORT,
> + NUMERIC_PROTO_UGID,
Do we need this necessarily?
> NUMERIC_ALL,
> };
>
> diff --git a/src/datatype.c b/src/datatype.c
> index 1518606..f9981a6 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -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.
> +
> + 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);
> diff --git a/src/expression.c b/src/expression.c
> index 1567870..bdbbd45 100644
> --- a/src/expression.c
> +++ b/src/expression.c
> @@ -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)
> diff --git a/src/main.c b/src/main.c
> index 6ba752b..1b28836 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -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.
> break;
> case OPT_STATELESS:
> stateless_output++;
> diff --git a/src/meta.c b/src/meta.c
> index cb7c136..9e8a987 100644
> --- a/src/meta.c
> +++ b/src/meta.c
> @@ -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);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-02-01 17:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-30 14:05 [PATCH nft] src: Always print range expressions numerically Elise Lennion
2017-02-01 17:58 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).