From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [libnftables PATCH] src: add fprintf functions
Date: Thu, 17 Oct 2013 12:11:01 +0200 [thread overview]
Message-ID: <20131017101101.GA10727@localhost> (raw)
In-Reply-To: <20131016171437.27537.1477.stgit@nfdev.cica.es>
Hi Arturo,
On Wed, Oct 16, 2013 at 07:14:38PM +0200, Arturo Borrero Gonzalez wrote:
> Now it's possible to print directly from libnftables to a file or other stream.
>
> In order to don't force any format, the caller must print '\n'.
>
> Signed-off-by: Arturo Borrero Gonzale <arturo.borrero.glez@gmail.com>
> ---
> include/libnftables/chain.h | 1 +
> include/libnftables/rule.h | 1 +
> include/libnftables/ruleset.h | 1 +
> include/libnftables/set.h | 2 ++
> include/libnftables/table.h | 1 +
> src/chain.c | 27 +++++++++++++++++++++++++++
> src/internal.h | 2 ++
> src/libnftables.map | 6 ++++++
> src/rule.c | 27 +++++++++++++++++++++++++++
> src/ruleset.c | 28 ++++++++++++++++++++++++++++
> src/set.c | 27 +++++++++++++++++++++++++++
> src/set_elem.c | 27 +++++++++++++++++++++++++++
> src/table.c | 27 +++++++++++++++++++++++++++
> 13 files changed, 177 insertions(+)
>
> diff --git a/include/libnftables/chain.h b/include/libnftables/chain.h
> index d3086ea..4b58f16 100644
> --- a/include/libnftables/chain.h
> +++ b/include/libnftables/chain.h
> @@ -58,6 +58,7 @@ enum nft_chain_parse_type {
>
> int nft_chain_parse(struct nft_chain *c, enum nft_chain_parse_type type, const char *data);
> int nft_chain_snprintf(char *buf, size_t size, struct nft_chain *t, uint32_t type, uint32_t flags);
> +int nft_chain_fprintf(FILE *fp, struct nft_chain *c, uint32_t type, uint32_t flags);
>
> struct nlmsghdr *nft_chain_nlmsg_build_hdr(char *buf, uint16_t cmd, uint16_t family, uint16_t type, uint32_t seq);
> int nft_chain_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_chain *t);
> diff --git a/include/libnftables/rule.h b/include/libnftables/rule.h
> index 9fba9c8..a35b700 100644
> --- a/include/libnftables/rule.h
> +++ b/include/libnftables/rule.h
> @@ -57,6 +57,7 @@ enum nft_rule_parse_type {
>
> int nft_rule_parse(struct nft_rule *r, enum nft_rule_parse_type type, const char *data);
> int nft_rule_snprintf(char *buf, size_t size, struct nft_rule *t, uint32_t type, uint32_t flags);
> +int nft_rule_fprintf(FILE *fp, struct nft_rule *r, uint32_t type, uint32_t flags);
>
> struct nlmsghdr *nft_rule_nlmsg_build_hdr(char *buf, uint16_t cmd, uint16_t family, uint16_t type, uint32_t seq);
> int nft_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_rule *t);
> diff --git a/include/libnftables/ruleset.h b/include/libnftables/ruleset.h
> index a4a1279..980276d 100644
> --- a/include/libnftables/ruleset.h
> +++ b/include/libnftables/ruleset.h
> @@ -37,6 +37,7 @@ enum nft_ruleset_parse_type {
>
> int nft_ruleset_parse(struct nft_ruleset *rs, enum nft_ruleset_parse_type type, const char *data);
> int nft_ruleset_snprintf(char *buf, size_t size, const struct nft_ruleset *rs, uint32_t type, uint32_t flags);
> +int nft_ruleset_fprintf(FILE *fp, const struct nft_ruleset *rs, uint32_t type, uint32_t flags);
>
> #ifdef __cplusplus
> } /* extern "C" */
> diff --git a/include/libnftables/set.h b/include/libnftables/set.h
> index e377826..2bddda4 100644
> --- a/include/libnftables/set.h
> +++ b/include/libnftables/set.h
> @@ -36,6 +36,7 @@ int nft_set_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s);
> int nft_set_elems_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set *s);
>
> int nft_set_snprintf(char *buf, size_t size, struct nft_set *s, uint32_t type, uint32_t flags);
> +int nft_set_fprintf(FILE *fp, struct nft_set *s, uint32_t type, uint32_t flags);
>
> struct nft_set_list;
>
> @@ -106,6 +107,7 @@ int nft_set_elem_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_set_elem *s)
>
> int nft_set_elem_parse(struct nft_set_elem *e, enum nft_set_parse_type type, const char *data);
> int nft_set_elem_snprintf(char *buf, size_t size, struct nft_set_elem *s, uint32_t type, uint32_t flags);
> +int nft_set_elem_fprintf(FILE *fp, struct nft_set_elem *se, uint32_t type, uint32_t flags);
>
> int nft_set_elem_foreach(struct nft_set *s, int (*cb)(struct nft_set_elem *e, void *data), void *data);
>
> diff --git a/include/libnftables/table.h b/include/libnftables/table.h
> index 66574cf..c412c43 100644
> --- a/include/libnftables/table.h
> +++ b/include/libnftables/table.h
> @@ -46,6 +46,7 @@ enum nft_table_parse_type {
>
> int nft_table_parse(struct nft_table *t, enum nft_table_parse_type type, const char *data);
> int nft_table_snprintf(char *buf, size_t size, struct nft_table *t, uint32_t type, uint32_t flags);
> +int nft_table_fprintf(FILE *fp, struct nft_table *t, uint32_t type, uint32_t flags);
>
> struct nlmsghdr *nft_table_nlmsg_build_hdr(char *buf, uint16_t cmd, uint16_t family, uint16_t type, uint32_t seq);
> int nft_table_nlmsg_parse(const struct nlmsghdr *nlh, struct nft_table *t);
> diff --git a/src/chain.c b/src/chain.c
> index f831479..b92b22f 100644
> --- a/src/chain.c
> +++ b/src/chain.c
> @@ -851,6 +851,33 @@ int nft_chain_snprintf(char *buf, size_t size, struct nft_chain *c,
> }
> EXPORT_SYMBOL(nft_chain_snprintf);
>
> +int nft_chain_fprintf(FILE *fp, struct nft_chain *c, uint32_t type,
> + uint32_t flags)
> +{
> + char _buf[NFT_SNPRINTF_BUFSIZ];
> + char *buf = _buf;
> + size_t bufsiz = sizeof(_buf);
> + int ret;
> +
> + ret = nft_chain_snprintf(buf, bufsiz, c, type, flags);
> + if (ret > NFT_SNPRINTF_BUFSIZ) {
> + buf = calloc(1, ret);
> + if (buf == NULL)
> + return -1;
> +
> + bufsiz = ret;
> + ret = nft_chain_snprintf(buf, bufsiz, c, type, flags);
> + }
> +
> + ret = fprintf(fp, "%s", buf);
> +
> + if (buf != _buf)
> + xfree(buf);
> +
> + return ret;
> +}
Most of this code looks very similar, I think you can refactorize it
by adding some internal function:
int nft_fprintf(FILE *fp, void *obj, uint32_t type, uint32_t flags,
size_t (*snprintf_cb)(char *but, size_t bufsiz, void *obj,
uint32_t type, uint32_t flags))
{
...
ret = snprintf_cb(...);
if (...) {
...
ret = snprintf_cb(...);
}
...
}
Thus, nft_chain_fprintf looks like:
int nft_chain_fprintf(...)
{
return nft_fprintf(..., nft_chain_snprintf);
}
One more comment below.
> +EXPORT_SYMBOL(nft_chain_fprintf);
> +
> struct nft_chain_list {
> struct list_head list;
> };
> diff --git a/src/internal.h b/src/internal.h
> index b29288a..e0f17ec 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -17,6 +17,8 @@
> #define BASE_DEC 10
> #define BASE_HEX 16
>
> +#define NFT_SNPRINTF_BUFSIZ 4096
> +
> enum nft_type {
> NFT_TYPE_U8,
> NFT_TYPE_U16,
> diff --git a/src/libnftables.map b/src/libnftables.map
> index 1223403..aa47714 100644
> --- a/src/libnftables.map
> +++ b/src/libnftables.map
> @@ -12,6 +12,7 @@ global:
> nft_table_attr_get_str;
> nft_table_parse;
> nft_table_snprintf;
> + nft_table_fprintf;
> nft_table_nlmsg_build_hdr;
> nft_table_nlmsg_build_payload;
> nft_table_nlmsg_parse;
> @@ -42,6 +43,7 @@ global:
> nft_chain_attr_get_str;
> nft_chain_parse;
> nft_chain_snprintf;
> + nft_chain_fprintf;
> nft_chain_nlmsg_build_hdr;
> nft_chain_nlmsg_build_payload;
> nft_chain_nlmsg_parse;
> @@ -71,6 +73,7 @@ global:
> nft_rule_attr_get_str;
> nft_rule_parse;
> nft_rule_snprintf;
> + nft_rule_fprintf;
> nft_rule_nlmsg_build_hdr;
> nft_rule_nlmsg_build_payload;
> nft_rule_nlmsg_parse;
> @@ -126,6 +129,7 @@ global:
> nft_set_nlmsg_parse;
> nft_set_parse;
> nft_set_snprintf;
> + nft_set_fprintf;
>
> nft_set_list_alloc;
> nft_set_list_free;
> @@ -157,6 +161,7 @@ global:
> nft_set_elem_nlmsg_parse;
> nft_set_elem_parse;
> nft_set_elem_snprintf;
> + nft_set_elem_fprinf;
>
> nft_set_elems_nlmsg_build_payload;
> nft_set_elems_nlmsg_parse;
> @@ -176,6 +181,7 @@ global:
> nft_ruleset_attr_get;
> nft_ruleset_parse;
> nft_ruleset_snprintf;
> + nft_ruleset_fprintf;
>
> local: *;
> };
> diff --git a/src/rule.c b/src/rule.c
> index 7f2bce6..68e733b 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -843,6 +843,33 @@ int nft_rule_snprintf(char *buf, size_t size, struct nft_rule *r,
> }
> EXPORT_SYMBOL(nft_rule_snprintf);
>
> +int nft_rule_fprintf(FILE *fp, struct nft_rule *r, uint32_t type,
> + uint32_t flags)
> +{
> + char _buf[NFT_SNPRINTF_BUFSIZ];
> + char *buf = _buf;
> + size_t bufsiz = sizeof(_buf);
> + int ret;
> +
> + ret = nft_rule_snprintf(buf, bufsiz, r, type, flags);
> + if (ret > NFT_SNPRINTF_BUFSIZ) {
> + buf = calloc(1, ret);
> + if (buf == NULL)
> + return -1;
> +
> + bufsiz = ret;
> + ret = nft_rule_snprintf(buf, bufsiz, r, type, flags);
> + }
> +
> + ret = fprintf(fp, "%s", buf);
> +
> + if (buf != _buf)
> + xfree(buf);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(nft_rule_fprintf);
> +
> int nft_rule_expr_foreach(struct nft_rule *r,
> int (*cb)(struct nft_rule_expr *e, void *data),
> void *data)
> diff --git a/src/ruleset.c b/src/ruleset.c
> index a7fbc33..54c3fc1 100644
> --- a/src/ruleset.c
> +++ b/src/ruleset.c
> @@ -811,3 +811,31 @@ int nft_ruleset_snprintf(char *buf, size_t size, const struct nft_ruleset *r,
> return -1;
> }
> EXPORT_SYMBOL(nft_ruleset_snprintf);
> +
> +int nft_ruleset_fprintf(FILE *fp, const struct nft_ruleset *rs, uint32_t type,
> + uint32_t flags)
> +{
> + char _buf[NFT_SNPRINTF_BUFSIZ];
> + char *buf = _buf;
> + size_t bufsiz = sizeof(_buf);
> + int ret;
> +
> + ret = nft_ruleset_snprintf(buf, bufsiz, rs, type, flags);
> + if (ret > NFT_SNPRINTF_BUFSIZ) {
For set and ruleset objects, this branch is very likely to happen,
since it is very likely that the buffer will be too small. To avoid
this, I think it's better to provide an optimized version for this
function that iterates over the list of objects and print them.
> + buf = calloc(1, ret);
> + if (buf == NULL)
> + return -1;
> +
> + bufsiz = ret;
> + ret = nft_ruleset_snprintf(buf, bufsiz, rs, type, flags);
> + }
> +
> + ret = fprintf(fp, "%s", buf);
> +
> + if (buf != _buf)
> + xfree(buf);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(nft_ruleset_fprintf);
next prev parent reply other threads:[~2013-10-17 10:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-16 17:14 [libnftables PATCH] src: add fprintf functions Arturo Borrero Gonzalez
2013-10-17 10:11 ` Pablo Neira Ayuso [this message]
2013-10-17 13:13 ` Arturo Borrero Gonzalez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131017101101.GA10727@localhost \
--to=pablo@netfilter.org \
--cc=arturo.borrero.glez@gmail.com \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).