[PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()

STALE1816d

4 messages, 2 authors, 2021-08-13 · open the first message on its own page

[PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()

From: Alexey Dobriyan <hidden>
Date: 2021-08-12 21:24:09

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

Signed-off-by: Alexey Dobriyan <redacted>
---

 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);
 }
 
 /**

Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-08-12 22:05:55

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?
quoted hunk
--- 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);
 }
 
 /**

Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()

From: Alexey Dobriyan <hidden>
Date: 2021-08-13 05:27:35

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:
quoted
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).
quoted
--- 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);
 }
 
 /**

Re: [PATCH net-next] netlink: gc useless variable in nlmsg_attrdata()

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-08-13 17:29:15

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:
quoted
On Fri, 13 Aug 2021 00:24:01 +0300 Alexey Dobriyan wrote:  
quoted
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 :(
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help