On Thu, Jun 15, 2017 at 01:01:57AM +0530, Varsha Rao wrote:
> diff --git a/include/datatype.h b/include/datatype.h
> index 04b7d88..748688e 100644
> --- a/include/datatype.h
> +++ b/include/datatype.h
> @@ -145,7 +145,8 @@ struct datatype {
> const char *desc;
> const struct datatype *basetype;
> const char *basefmt;
> - void (*print)(const struct expr *expr);
> + void (*print)(const struct expr *expr,
> + struct print_ctx *ct);
Could you use:
struct print_ctx *pctx
instead?
'ct' usually refers to the connection tracking system (ct) in our
codebase, so I would prefer we have a different variable name.
This applies everywhere in this patch.
> struct error_record *(*parse)(const struct expr *sym,
> struct expr **res);
> const struct symbol_table *sym_tbl;
> @@ -157,7 +158,7 @@ extern const struct datatype *datatype_lookup_byname(const char *name);
>
> extern struct error_record *symbol_parse(const struct expr *sym,
> struct expr **res);
> -extern void datatype_print(const struct expr *expr);
> +extern void datatype_print(const struct expr *expr, struct print_ctx *ct);
>
> static inline bool datatype_equal(const struct datatype *d1,
> const struct datatype *d2)
> @@ -205,7 +206,8 @@ extern struct error_record *symbolic_constant_parse(const struct expr *sym,
> const struct symbol_table *tbl,
> struct expr **res);
> extern void symbolic_constant_print(const struct symbol_table *tbl,
> - const struct expr *expr, bool quotes);
> + const struct expr *expr, bool quotes,
> + struct print_ctx *ct);
> extern void symbol_table_print(const struct symbol_table *tbl,
> const struct datatype *dtype,
> enum byteorder byteorder);
> diff --git a/include/expression.h b/include/expression.h
> index 9ba87e8..80f91d0 100644
> --- a/include/expression.h
> +++ b/include/expression.h
> @@ -157,7 +157,8 @@ struct expr_ops {
> void (*set_type)(const struct expr *expr,
> const struct datatype *dtype,
> enum byteorder byteorder);
> - void (*print)(const struct expr *expr);
> + void (*print)(const struct expr *expr,
> + struct print_ctx *ct);
> bool (*cmp)(const struct expr *e1,
> const struct expr *e2);
> void (*pctx_update)(struct proto_ctx *ctx,
> @@ -330,7 +331,7 @@ extern struct expr *expr_alloc(const struct location *loc,
> extern struct expr *expr_clone(const struct expr *expr);
> extern struct expr *expr_get(struct expr *expr);
> extern void expr_free(struct expr *expr);
> -extern void expr_print(const struct expr *expr);
> +extern void expr_print(const struct expr *expr, struct print_ctx *ct);
> extern bool expr_cmp(const struct expr *e1, const struct expr *e2);
> extern void expr_describe(const struct expr *expr);
>
> @@ -410,7 +411,7 @@ extern struct expr *list_expr_alloc(const struct location *loc);
>
> extern struct expr *set_expr_alloc(const struct location *loc);
> extern int set_to_intervals(struct list_head *msgs, struct set *set,
> - struct expr *init, bool add);
> + struct expr *init, bool add, struct print_ctx *ct);
Why do we need this? I think we only need print_ctx for *_print()
functions.
> extern void interval_map_decompose(struct expr *set);
>
> extern struct expr *mapping_expr_alloc(const struct location *loc,
> diff --git a/include/netlink.h b/include/netlink.h
> index 81538ff..0c7cd90 100644
> --- a/include/netlink.h
> +++ b/include/netlink.h
> @@ -213,6 +213,7 @@ struct netlink_mon_handler {
> struct netlink_ctx *ctx;
> const struct location *loc;
> bool cache_needed;
> + struct print_ctx *ct;
I think you can place this in 'struct netlink_ctx', see more detailed
comment below.
> diff --git a/include/nftables.h b/include/nftables.h
> index 6f54155..1c747d6 100644
> --- a/include/nftables.h
> +++ b/include/nftables.h
> @@ -24,12 +24,16 @@ enum debug_level {
>
> #define INCLUDE_PATHS_MAX 16
>
> +struct print_ctx {
> + unsigned int numeric_output;
> + unsigned int stateless_output;
> + unsigned int ip2name_output;
> + unsigned int handle_output;
You can probably remove the trailing "_output" now that this is inside
struct print_ctx, it is obvious they refer to printing toggles.
> +};
> +
> extern unsigned int max_errors;
> -extern unsigned int numeric_output;
> -extern unsigned int stateless_output;
> -extern unsigned int ip2name_output;
> -extern unsigned int handle_output;
> extern unsigned int debug_level;
> +
> extern const char *include_paths[INCLUDE_PATHS_MAX];
>
> enum nftables_exit_codes {
> @@ -107,6 +111,7 @@ struct input_descriptor {
>
> struct parser_state;
>
> -int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs);
> +int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs,
> + struct print_ctx *ct);
>
> #endif /* NFTABLES_NFTABLES_H */
> diff --git a/include/rule.h b/include/rule.h
> index fb46064..48cb5f1 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -195,7 +195,7 @@ struct rule {
> extern struct rule *rule_alloc(const struct location *loc,
> const struct handle *h);
> extern void rule_free(struct rule *rule);
> -extern void rule_print(const struct rule *rule);
> +extern void rule_print(const struct rule *rule, struct print_ctx *ct);
> extern struct rule *rule_lookup(const struct chain *chain, uint64_t handle);
>
> /**
> @@ -244,7 +244,7 @@ extern void set_add_hash(struct set *set, struct table *table);
> extern struct set *set_lookup(const struct table *table, const char *name);
> extern struct set *set_lookup_global(uint32_t family, const char *table,
> const char *name);
> -extern void set_print(const struct set *set);
> +extern void set_print(const struct set *set, struct print_ctx *ct);
> extern void set_print_plain(const struct set *s);
>
> #include <statement.h>
> @@ -292,7 +292,7 @@ void obj_free(struct obj *obj);
> void obj_add_hash(struct obj *obj, struct table *table);
> struct obj *obj_lookup(const struct table *table, const char *name,
> uint32_t type);
> -void obj_print(const struct obj *n);
> +void obj_print(const struct obj *n, struct print_ctx *ct);
> void obj_print_plain(const struct obj *obj);
> const char *obj_type_name(uint32_t type);
>
> @@ -475,7 +475,8 @@ extern int cmd_evaluate(struct eval_ctx *ctx, struct cmd *cmd);
> extern struct error_record *rule_postprocess(struct rule *rule);
>
> struct netlink_ctx;
> -extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd);
> +extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd,
> + struct print_ctx *ct);
You can place this in netlink_ctx so...
struct netlink_ctx {
...
struct print_ctx *pctx;
};
So we can make this patch smaller, you don't need to update that many
functions.
> extern int cache_update(enum cmd_ops cmd, struct list_head *msgs);
> extern void cache_flush(void);
> diff --git a/include/statement.h b/include/statement.h
> index 317d53e..d6ebc01 100644
> --- a/include/statement.h
> +++ b/include/statement.h
> @@ -261,7 +261,8 @@ struct stmt_ops {
> enum stmt_types type;
> const char *name;
> void (*destroy)(struct stmt *stmt);
> - void (*print)(const struct stmt *stmt);
> + void (*print)(const struct stmt *stmt,
> + struct print_ctx *ct);
> };
>
> enum stmt_flags {
> @@ -312,7 +313,7 @@ extern struct stmt *stmt_alloc(const struct location *loc,
> int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt);
> extern void stmt_free(struct stmt *stmt);
> extern void stmt_list_free(struct list_head *list);
> -extern void stmt_print(const struct stmt *stmt);
> +extern void stmt_print(const struct stmt *stmt, struct print_ctx *ct);
>
> const char *get_rate(uint64_t byte_rate, uint64_t *rate);
>
> diff --git a/src/cli.c b/src/cli.c
> index a74411a..c91eced 100644
> --- a/src/cli.c
> +++ b/src/cli.c
> @@ -129,7 +129,7 @@ static void cli_complete(char *line)
>
> parser_init(state, &msgs);
> scanner_push_buffer(scanner, &indesc_cli, line);
> - nft_run(scanner, state, &msgs);
> + nft_run(scanner, state, &msgs, NULL);
I suggest you pass it here as global. Look, the cli code (interactive
mode that you exercise via -i) will not go into libnftables, so it's
fine to to keep it global.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
