| Message ID | YRWRcbWR45+zF9mD@localhost.localdomain (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] netlink: gc useless variable in nlmsg_attrdata() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 3408 this patch: 3408 |
| netdev/kdoc | success | Errors and warnings before: 14 this patch: 14 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 9 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 3455 this patch: 3455 |
| netdev/header_inline | success | Link |
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); > } > > /**
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); > > } > > > > /** >
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 :(
--- 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); } /**
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(-)