All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()
@ 2021-08-12 21:24 Alexey Dobriyan
  2021-08-12 22:05 ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2021-08-12 21:24 UTC (permalink / raw)
  To: davem; +Cc: netdev

Kernel permits pointer arithmetic on "void*" so might as well use it
without casts back and forth.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/net/netlink.h |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -587,8 +587,7 @@ static inline int nlmsg_len(const struct nlmsghdr *nlh)
 static inline struct nlattr *nlmsg_attrdata(const struct nlmsghdr *nlh,
 					    int hdrlen)
 {
-	unsigned char *data = nlmsg_data(nlh);
-	return (struct nlattr *) (data + NLMSG_ALIGN(hdrlen));
+	return nlmsg_data(nlh) + NLMSG_ALIGN(hdrlen);
 }
 
 /**

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()
  2021-08-12 21:24 [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata() Alexey Dobriyan
@ 2021-08-12 22:05 ` Jakub Kicinski
  2021-08-13  5:27   ` Alexey Dobriyan
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2021-08-12 22:05 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: davem, netdev

On Fri, 13 Aug 2021 00:24:01 +0300 Alexey Dobriyan wrote:
> Kernel permits pointer arithmetic on "void*" so might as well use it
> without casts back and forth.

But why change existing code? It's perfectly fine, right?

> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -587,8 +587,7 @@ static inline int nlmsg_len(const struct nlmsghdr *nlh)
>  static inline struct nlattr *nlmsg_attrdata(const struct nlmsghdr *nlh,
>  					    int hdrlen)
>  {
> -	unsigned char *data = nlmsg_data(nlh);
> -	return (struct nlattr *) (data + NLMSG_ALIGN(hdrlen));
> +	return nlmsg_data(nlh) + NLMSG_ALIGN(hdrlen);
>  }
>  
>  /**


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()
  2021-08-12 22:05 ` Jakub Kicinski
@ 2021-08-13  5:27   ` Alexey Dobriyan
  2021-08-13 17:29     ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2021-08-13  5:27 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev

On Thu, Aug 12, 2021 at 03:05:52PM -0700, Jakub Kicinski wrote:
> On Fri, 13 Aug 2021 00:24:01 +0300 Alexey Dobriyan wrote:
> > Kernel permits pointer arithmetic on "void*" so might as well use it
> > without casts back and forth.
> 
> But why change existing code? It's perfectly fine, right?

It is harder to read (marginally of course).

> > --- a/include/net/netlink.h
> > +++ b/include/net/netlink.h
> > @@ -587,8 +587,7 @@ static inline int nlmsg_len(const struct nlmsghdr *nlh)
> >  static inline struct nlattr *nlmsg_attrdata(const struct nlmsghdr *nlh,
> >  					    int hdrlen)
> >  {
> > -	unsigned char *data = nlmsg_data(nlh);
> > -	return (struct nlattr *) (data + NLMSG_ALIGN(hdrlen));
> > +	return nlmsg_data(nlh) + NLMSG_ALIGN(hdrlen);
> >  }
> >  
> >  /**
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()
  2021-08-13  5:27   ` Alexey Dobriyan
@ 2021-08-13 17:29     ` Jakub Kicinski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2021-08-13 17:29 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: davem, netdev

On Fri, 13 Aug 2021 08:27:30 +0300 Alexey Dobriyan wrote:
> On Thu, Aug 12, 2021 at 03:05:52PM -0700, Jakub Kicinski wrote:
> > On Fri, 13 Aug 2021 00:24:01 +0300 Alexey Dobriyan wrote:  
> > > Kernel permits pointer arithmetic on "void*" so might as well use it
> > > without casts back and forth.  
> > 
> > But why change existing code? It's perfectly fine, right?  
> 
> It is harder to read (marginally of course).

TBH I prefer the current code, I don't have to wonder what type
nlmsg_data() returns and whether it's okay to do void* arithmetic 
in this header (if it's uAPI). Sorry :(

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()
@ 2021-08-13  2:23 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2021-08-13  2:23 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2633 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <YRWRcbWR45+zF9mD@localhost.localdomain>
References: <YRWRcbWR45+zF9mD@localhost.localdomain>
TO: Alexey Dobriyan <adobriyan@gmail.com>
TO: davem(a)davemloft.net
CC: netdev(a)vger.kernel.org

Hi Alexey,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Alexey-Dobriyan/netlink-gc-useless-variable-in-nlmsg_attrdata/20210813-052557
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git bed5a942e27e1df67250e27e1f2eb5ea2d4cc362
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: i386-randconfig-m021-20210812 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
net/ipv4/inet_diag.c:219 inet_diag_parse_attrs() warn: potential spectre issue 'req_nlas' [w] (local cap)

Old smatch warnings:
include/linux/cgroup-defs.h:817 sock_cgroup_classid() warn: maybe use && instead of &

vim +/req_nlas +219 net/ipv4/inet_diag.c

cb2050a7b8131a9 Xin Long     2016-04-14  205  
d5e4d0a5e692a94 Eric Dumazet 2020-09-21  206  static int inet_diag_parse_attrs(const struct nlmsghdr *nlh, int hdrlen,
3f935c75eb52dd9 Paolo Abeni  2020-07-09  207  				 struct nlattr **req_nlas)
3f935c75eb52dd9 Paolo Abeni  2020-07-09  208  {
3f935c75eb52dd9 Paolo Abeni  2020-07-09  209  	struct nlattr *nla;
3f935c75eb52dd9 Paolo Abeni  2020-07-09  210  	int remaining;
3f935c75eb52dd9 Paolo Abeni  2020-07-09  211  
3f935c75eb52dd9 Paolo Abeni  2020-07-09  212  	nlmsg_for_each_attr(nla, nlh, hdrlen, remaining) {
3f935c75eb52dd9 Paolo Abeni  2020-07-09  213  		int type = nla_type(nla);
3f935c75eb52dd9 Paolo Abeni  2020-07-09  214  
d5e4d0a5e692a94 Eric Dumazet 2020-09-21  215  		if (type == INET_DIAG_REQ_PROTOCOL && nla_len(nla) != sizeof(u32))
d5e4d0a5e692a94 Eric Dumazet 2020-09-21  216  			return -EINVAL;
d5e4d0a5e692a94 Eric Dumazet 2020-09-21  217  
3f935c75eb52dd9 Paolo Abeni  2020-07-09  218  		if (type < __INET_DIAG_REQ_MAX)
3f935c75eb52dd9 Paolo Abeni  2020-07-09 @219  			req_nlas[type] = nla;
3f935c75eb52dd9 Paolo Abeni  2020-07-09  220  	}
d5e4d0a5e692a94 Eric Dumazet 2020-09-21  221  	return 0;
3f935c75eb52dd9 Paolo Abeni  2020-07-09  222  }
3f935c75eb52dd9 Paolo Abeni  2020-07-09  223  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35148 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-13 17:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 21:24 [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata() Alexey Dobriyan
2021-08-12 22:05 ` Jakub Kicinski
2021-08-13  5:27   ` Alexey Dobriyan
2021-08-13 17:29     ` Jakub Kicinski
2021-08-13  2:23 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.