| Message ID | 20170819152420.22563-14-eric@regit.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Sat, Aug 19, 2017 at 05:24:15PM +0200, Eric Leblond wrote: > This function allows user to set his own printing function. It is > still dependant of the format used by nft but at least it can be > redirected easily. I'm looking at this patch... > Signed-off-by: Eric Leblond <eric@regit.org> > --- > include/nftables/nftables.h | 3 +++ > src/libnftables.c | 9 +++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/nftables/nftables.h b/include/nftables/nftables.h > index b902cbd..935d0db 100644 > --- a/include/nftables/nftables.h > +++ b/include/nftables/nftables.h > @@ -26,6 +26,9 @@ void nft_global_deinit(void); > > struct nft_ctx *nft_context_new(void); > void nft_context_free(struct nft_ctx *nft); > +void nft_context_set_print_func(struct nft_ctx *nft, > + int (*print)(void *ctx, const char *fmt, ...), > + void *ctx); > > int nft_run_command_from_buffer(struct nft_ctx *nft, > char *buf, size_t buflen); > diff --git a/src/libnftables.c b/src/libnftables.c > index 7209885..f0decae 100644 > --- a/src/libnftables.c > +++ b/src/libnftables.c > @@ -86,6 +86,15 @@ struct nft_ctx *nft_context_new(void) > return ctx; > } > > +void nft_context_set_print_func(struct nft_ctx *nft, > + int (*print)(void *ctx, const char *fmt, ...), > + void *ctx) Can we have a strict type here instead of void *ctx? I mean, if ctx is always going to be a file descriptor, I would prefer this is a specific type. Or are you envisioning any real use of this generic type? If we cannot forecast anything reasonable, then a strict type is the way to go IMO. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, 2017-08-25 at 11:59 +0200, Pablo Neira Ayuso wrote: > On Sat, Aug 19, 2017 at 05:24:15PM +0200, Eric Leblond wrote: > > This function allows user to set his own printing function. It is > > still dependant of the format used by nft but at least it can be > > redirected easily. > > I'm looking at this patch... > > > Signed-off-by: Eric Leblond <eric@regit.org> > > --- > > include/nftables/nftables.h | 3 +++ > > src/libnftables.c | 9 +++++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/include/nftables/nftables.h > > b/include/nftables/nftables.h > > index b902cbd..935d0db 100644 > > --- a/include/nftables/nftables.h > > +++ b/include/nftables/nftables.h > > @@ -26,6 +26,9 @@ void nft_global_deinit(void); > > > > struct nft_ctx *nft_context_new(void); > > void nft_context_free(struct nft_ctx *nft); > > +void nft_context_set_print_func(struct nft_ctx *nft, > > + int (*print)(void *ctx, const char > > *fmt, ...), > > + void *ctx); > > > > int nft_run_command_from_buffer(struct nft_ctx *nft, > > char *buf, size_t buflen); > > diff --git a/src/libnftables.c b/src/libnftables.c > > index 7209885..f0decae 100644 > > --- a/src/libnftables.c > > +++ b/src/libnftables.c > > @@ -86,6 +86,15 @@ struct nft_ctx *nft_context_new(void) > > return ctx; > > } > > > > +void nft_context_set_print_func(struct nft_ctx *nft, > > + int (*print)(void *ctx, const char > > *fmt, ...), > > + void *ctx) > > Can we have a strict type here instead of void *ctx? I mean, if ctx > is > always going to be a file descriptor, I would prefer this is a > specific type. > > Or are you envisioning any real use of this generic type? If we > cannot > forecast anything reasonable, then a strict type is the way to go > IMO. Yes, it can be any internal structure from the user application. For instance, it can be a structure storing logging information, that accumulate the buffer and do a line by line output in a GUI. Writing that I realize that it would be better at least for high level function to provide a complete buffer containing the error to the user instead of that. Maybe it could be the default function and we provide that function for some corner cases ? ++
On Fri, Aug 25, 2017 at 01:49:56PM +0200, Eric Leblond wrote: > Hi, > > On Fri, 2017-08-25 at 11:59 +0200, Pablo Neira Ayuso wrote: > > On Sat, Aug 19, 2017 at 05:24:15PM +0200, Eric Leblond wrote: > > > This function allows user to set his own printing function. It is > > > still dependant of the format used by nft but at least it can be > > > redirected easily. > > > > I'm looking at this patch... > > > > > Signed-off-by: Eric Leblond <eric@regit.org> > > > --- > > > include/nftables/nftables.h | 3 +++ > > > src/libnftables.c | 9 +++++++++ > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/include/nftables/nftables.h > > > b/include/nftables/nftables.h > > > index b902cbd..935d0db 100644 > > > --- a/include/nftables/nftables.h > > > +++ b/include/nftables/nftables.h > > > @@ -26,6 +26,9 @@ void nft_global_deinit(void); > > > > > > struct nft_ctx *nft_context_new(void); > > > void nft_context_free(struct nft_ctx *nft); > > > +void nft_context_set_print_func(struct nft_ctx *nft, > > > + int (*print)(void *ctx, const char > > > *fmt, ...), > > > + void *ctx); > > > > > > int nft_run_command_from_buffer(struct nft_ctx *nft, > > > char *buf, size_t buflen); > > > diff --git a/src/libnftables.c b/src/libnftables.c > > > index 7209885..f0decae 100644 > > > --- a/src/libnftables.c > > > +++ b/src/libnftables.c > > > @@ -86,6 +86,15 @@ struct nft_ctx *nft_context_new(void) > > > return ctx; > > > } > > > > > > +void nft_context_set_print_func(struct nft_ctx *nft, > > > + int (*print)(void *ctx, const char > > > *fmt, ...), > > > + void *ctx) > > > > Can we have a strict type here instead of void *ctx? I mean, if ctx > > is > > always going to be a file descriptor, I would prefer this is a > > specific type. > > > > Or are you envisioning any real use of this generic type? If we > > cannot > > forecast anything reasonable, then a strict type is the way to go > > IMO. > > Yes, it can be any internal structure from the user application. For > instance, it can be a structure storing logging information, that > accumulate the buffer and do a line by line output in a GUI. I don't think we can do snprintf() with this interface, or any other printing to buffer with this approach. We would need to update the libnftables codebase to keep track of offsets and so on. > Writing that I realize that it would be better at least for high level > function to provide a complete buffer containing the error to the user > instead of that. Maybe it could be the default function and we provide > that function for some corner cases ? I'd suggest we start with something simple that suit your specific needs, and at the same make sure we can extend such API to cover the buffer like and file description printing, which are the two common cases I can think of. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 30, 2017 at 12:46:17PM +0200, Pablo Neira Ayuso wrote: > On Fri, Aug 25, 2017 at 01:49:56PM +0200, Eric Leblond wrote: > > On Fri, 2017-08-25 at 11:59 +0200, Pablo Neira Ayuso wrote: > > > On Sat, Aug 19, 2017 at 05:24:15PM +0200, Eric Leblond wrote: > > > > diff --git a/src/libnftables.c b/src/libnftables.c > > > > index 7209885..f0decae 100644 > > > > --- a/src/libnftables.c > > > > +++ b/src/libnftables.c > > > > @@ -86,6 +86,15 @@ struct nft_ctx *nft_context_new(void) > > > > return ctx; > > > > } > > > > > > > > +void nft_context_set_print_func(struct nft_ctx *nft, > > > > + int (*print)(void *ctx, const char > > > > *fmt, ...), > > > > + void *ctx) > > > > > > Can we have a strict type here instead of void *ctx? I mean, if ctx > > > is > > > always going to be a file descriptor, I would prefer this is a > > > specific type. > > > > > > Or are you envisioning any real use of this generic type? If we > > > cannot > > > forecast anything reasonable, then a strict type is the way to go > > > IMO. > > > > Yes, it can be any internal structure from the user application. For > > instance, it can be a structure storing logging information, that > > accumulate the buffer and do a line by line output in a GUI. > > I don't think we can do snprintf() with this interface, or any other > printing to buffer with this approach. > > We would need to update the libnftables codebase to keep track of > offsets and so on. Forget this. We can indeed use this API to implement buffer like printing. But I would prefer strict typing for this API, so we provide to functions, one to print to buffer and another to print to descriptors, if this is what you need. Otherwise, just provide a function that does exactly what you need right now, with strict typing. If someone else needs a new function to do something else, then let's wait for them to come and ask for it. OK? -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/nftables/nftables.h b/include/nftables/nftables.h index b902cbd..935d0db 100644 --- a/include/nftables/nftables.h +++ b/include/nftables/nftables.h @@ -26,6 +26,9 @@ void nft_global_deinit(void); struct nft_ctx *nft_context_new(void); void nft_context_free(struct nft_ctx *nft); +void nft_context_set_print_func(struct nft_ctx *nft, + int (*print)(void *ctx, const char *fmt, ...), + void *ctx); int nft_run_command_from_buffer(struct nft_ctx *nft, char *buf, size_t buflen); diff --git a/src/libnftables.c b/src/libnftables.c index 7209885..f0decae 100644 --- a/src/libnftables.c +++ b/src/libnftables.c @@ -86,6 +86,15 @@ struct nft_ctx *nft_context_new(void) return ctx; } +void nft_context_set_print_func(struct nft_ctx *nft, + int (*print)(void *ctx, const char *fmt, ...), + void *ctx) +{ + if (nft) { + nft->output.print = print; + nft->output.ctx = ctx; + } +} void nft_context_free(struct nft_ctx *nft) {
This function allows user to set his own printing function. It is still dependant of the format used by nft but at least it can be redirected easily. Signed-off-by: Eric Leblond <eric@regit.org> --- include/nftables/nftables.h | 3 +++ src/libnftables.c | 9 +++++++++ 2 files changed, 12 insertions(+)