Quoting Dmitry V. Levin (2016-05-14 23:27:36)
> On Fri, May 13, 2016 at 12:02:37PM +0000, Fabien Siron wrote:
> >  #include <linux/sock_diag.h>
> >  #include <linux/inet_diag.h>
> >  #include <linux/unix_diag.h>
> > +#include <linux/netlink_diag.h>
> 
> This header first appeared in linux v3.10-rc1, but strace can be built
> with older kernel headers.
> 
> In similar cases of linux/sock_diag.h, linux/inet_diag.h, and
> linux/unix_diag.h we have stripped down replacement headers.

Ah yes, I didn't notice them.

> 
> >  #include <linux/rtnetlink.h>
> > +# include "xlat/netlink_protocols.h"
> 
> No need to indent.
> 
> >  
> >  #if !defined NETLINK_SOCK_DIAG && defined NETLINK_INET_DIAG
> >  # define NETLINK_SOCK_DIAG NETLINK_INET_DIAG
> > @@ -335,6 +337,60 @@ unix_parse_response(const char *proto_name, const void 
> > *data,
> >  }
> >  
> >  static bool
> > +netlink_send_query(const int fd, const unsigned long inode)
> > +{
> > +     struct {
> > +             const struct nlmsghdr nlh;
> > +             const struct netlink_diag_req idr;
> 
> In inet_send_query, idr has type struct inet_diag_req_v2.
> If you follow this naming scheme, idr should be called ndr.
> 
> > +     } req = {
> > +             .nlh = {
> > +                     .nlmsg_len = sizeof(req),
> > +                     .nlmsg_type = SOCK_DIAG_BY_FAMILY,
> > +                     .nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST
> > +             },
> > +             .idr = {
> > +                     .sdiag_family = AF_NETLINK,
> > +                     .sdiag_protocol = NDIAG_PROTO_ALL,
> > +                     .ndiag_ino = (unsigned) inode,
> 
> Isn't implicit cast enough?
> 
> BTW, inode lookup is not implemented anyway, so like in case of
> unix_send_query, .ndiag_ino initialization has no practical purpose.

Do you suggest to remove .ndiag_ino initialization?

> 
> > ...
> 
> I've tried this with "ip l" command, and it printed
> <NETLINK:[NETLINK_ROUTE]>, which looks redundant.
> 
> Wouldn't it be more useful to print just netlink_proto but also add more
> netlink specific information, e.g. <NETLINK_ROUTE:[ndiag_portid]>?

I agree.

> 
> > ...
> 
> This looks OK.  Could you add a test similar to already existing net-yy-*
> tests (not necessarily with so many different syscalls)?
> 
Sure.

Cheers,
--
Fabien Siron

------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to