Page MenuHomeFreeBSD

netlink: Do not cast to int in NLMSG_HDRLEN and _NLMSG_LEN
Needs ReviewPublic

Authored by arrowd on Oct 29 2024, 6:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 6, 9:58 AM
Unknown Object (File)
Wed, Dec 4, 7:48 PM
Unknown Object (File)
Tue, Dec 3, 10:30 PM
Unknown Object (File)
Tue, Dec 3, 6:57 PM
Unknown Object (File)
Fri, Nov 22, 11:37 PM
Unknown Object (File)
Fri, Nov 22, 4:36 AM
Unknown Object (File)
Wed, Nov 13, 10:59 AM
Unknown Object (File)
Nov 8 2024, 7:08 AM
Subscribers

Details

Reviewers
glebius
melifaro
jhb
Group Reviewers
Src Committers
Test Plan

make buildkernel passes

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 60252
Build 57136: arc lint + arc unit

Event Timeline

Some other cleanups to consider as separate commits: Just use the userspace version of NLMSG_ALIGNTO and NLMSG_ALIGN always (so move them above the #ifndef _KERNEL). Also, replace roundup2() with just using __align_up() from <sys/cdefs.h> unconditionally.

I'm also not convinced that NLMSG_HDRLEN isn't in practice the same for both userspace and the kernel.

sys/netlink/netlink.h
216

sizeof() already returns a size_t, so you can just remove this cast instead

229

I'm not sure you need this cast either

arrowd marked 2 inline comments as done.

Address comments

arrowd retitled this revision from netlink: Cast to (size_t) instead of (int) in NLMSG_HDRLEN and _NLMSG_LEN to netlink: Do not cast to int in NLMSG_HDRLEN and _NLMSG_LEN.Oct 31 2024, 4:49 PM

I'm fine with doing all this at once, though you might want to update the log to mention using __align_up and reducing duplication, etc.

sys/netlink/netlink.h
229

You can use tabs to align the macro value now while you are here.

This revision is now accepted and ready to land.Oct 31 2024, 4:52 PM
In D47333#1080149, @jhb wrote:

I'm fine with doing all this at once, though you might want to update the log to mention using __align_up and reducing duplication, etc.

I did it as three commits, see the "Commits" tab.

arrowd marked an inline comment as done.
  • Align with tabs
This revision now requires review to proceed.Oct 31 2024, 4:58 PM

Ah, ok. I tend to upload individual commits as a stack. Still LGTM, thanks!

Let’s make sure that the constants /macro still has the same type as in Linux and check that, for example, net/bird2 compiles without additional warnings after this change

net/bird2 compiles without additional warnings after this change

Checked that, it builds with no new warnings.