[netsniff-ng] Re: [PATCH 1/4] netsniff-ng: nlmsg: Resolve genl family name

[Vadim Kochan]

On Tue, Dec 08, 2015 at 05:00:52PM +0100, Tobias Klauser wrote:
> On 2015-11-30 at 01:05:04 +0100, Vadim Kochan  wrote:
> > Print name of resolved genl family name by type
> 
> This patch does quite a bit more than the description says (i.e. the
> init/uninit hooks). Please be a bit more verbose in your patch
> descriptions.
> 
> > Signed-off-by: Vadim Kochan 
> > ---
> >  dissector_netlink.c |  3 ++
> >  proto.h | 15 ++
> >  proto_nlmsg.c   | 82 
> > +
> >  protos.h|  2 +-
> >  4 files changed, 96 insertions(+), 6 deletions(-)
> > 
> > diff --git a/dissector_netlink.c b/dissector_netlink.c
> > index 2b23a99..b4de112 100644
> > --- a/dissector_netlink.c
> > +++ b/dissector_netlink.c
> > @@ -19,10 +19,13 @@ static inline void dissector_init_exit(int type)
> >  
> >  void dissector_init_netlink(int fnttype)
> >  {
> > +   proto_ops_init(_ops);
> > +
> > dissector_init_entry(fnttype);
> > dissector_init_exit(fnttype);
> >  }
> >  
> >  void dissector_cleanup_netlink(void)
> >  {
> > +   proto_ops_uninit(_ops);
> >  }
> > diff --git a/proto.h b/proto.h
> > index 0cc1fd8..03a07e2 100644
> > --- a/proto.h
> > +++ b/proto.h
> > @@ -10,6 +10,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> 
> What is this needed for?
> 
> >  
> >  #include "tprintf.h"
> >  
> > @@ -20,6 +21,8 @@ struct protocol {
> > const unsigned int key;
> > void (*print_full)(struct pkt_buff *pkt);
> > void (*print_less)(struct pkt_buff *pkt);
> > +   void (*init)(void);
> > +   void (*uninit)(void);
> 
> I don't think the very specific case of dissecting genl family messages
> deserves the introduction of these hooks. How about just doing the init
> work the first time the genl stuff is actually used?

Yes, init can be done on demand within proto_nlmsg.c module by 1st, but what 
about uninit ?
May be at least uninit/cleanup/exit hook might be added only which will
close genl socket if it is not NULL ?

Regards,

> 
> Sorry for the brevity, I currently have a very limited bandwidth to
> review patches...

-- 
You received this message because you are subscribed to the Google Groups 
"netsniff-ng" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[netsniff-ng] Re: [PATCH 1/4] netsniff-ng: nlmsg: Resolve genl family name

[Tobias Klauser]

On 2015-11-30 at 01:05:04 +0100, Vadim Kochan  wrote:
> Print name of resolved genl family name by type

This patch does quite a bit more than the description says (i.e. the
init/uninit hooks). Please be a bit more verbose in your patch
descriptions.

> Signed-off-by: Vadim Kochan 
> ---
>  dissector_netlink.c |  3 ++
>  proto.h | 15 ++
>  proto_nlmsg.c   | 82 
> +
>  protos.h|  2 +-
>  4 files changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/dissector_netlink.c b/dissector_netlink.c
> index 2b23a99..b4de112 100644
> --- a/dissector_netlink.c
> +++ b/dissector_netlink.c
> @@ -19,10 +19,13 @@ static inline void dissector_init_exit(int type)
>  
>  void dissector_init_netlink(int fnttype)
>  {
> + proto_ops_init(_ops);
> +
>   dissector_init_entry(fnttype);
>   dissector_init_exit(fnttype);
>  }
>  
>  void dissector_cleanup_netlink(void)
>  {
> + proto_ops_uninit(_ops);
>  }
> diff --git a/proto.h b/proto.h
> index 0cc1fd8..03a07e2 100644
> --- a/proto.h
> +++ b/proto.h
> @@ -10,6 +10,7 @@
>  
>  #include 
>  #include 
> +#include 

What is this needed for?

>  
>  #include "tprintf.h"
>  
> @@ -20,6 +21,8 @@ struct protocol {
>   const unsigned int key;
>   void (*print_full)(struct pkt_buff *pkt);
>   void (*print_less)(struct pkt_buff *pkt);
> + void (*init)(void);
> + void (*uninit)(void);

I don't think the very specific case of dissecting genl family messages
deserves the introduction of these hooks. How about just doing the init
work the first time the genl stuff is actually used?

Sorry for the brevity, I currently have a very limited bandwidth to
review patches...

-- 
You received this message because you are subscribed to the Google Groups 
"netsniff-ng" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.