Page MenuHomeFreeBSD

IfAPI: Style cleanup
ClosedPublic

Authored by jhibbits on Feb 10 2023, 9:35 PM.
Tags
None
Referenced Files
F102843354: D38499.diff
Sun, Nov 17, 9:42 PM
Unknown Object (File)
Wed, Nov 6, 3:47 PM
Unknown Object (File)
Mon, Nov 4, 4:39 PM
Unknown Object (File)
Sep 30 2024, 1:40 PM
Unknown Object (File)
Sep 30 2024, 1:33 PM
Unknown Object (File)
Sep 30 2024, 1:33 PM
Unknown Object (File)
Sep 30 2024, 1:32 PM
Unknown Object (File)
Sep 30 2024, 1:32 PM

Details

Reviewers
kib
glebius
Group Reviewers
network
Commits
rGaac2d19d9385: IfAPI: Style cleanup
Summary

Casts to (struct ifnet *) made sense when if_t was a void *, but
since it's a struct ifnet * it no longer makes sense. Also remove
pointless conditionals in if_setcapenable().

Sponsored by: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 49764
Build 46654: arc lint + arc unit

Event Timeline

A lot of functions return int and that boils down to return (0);. Do you intend to make the fail in some way in the future? If yes, could you please describe what are the expected failure modes?

sys/net/if.c
4197–4198

Remove no longer needed blank line as well?

4211–4212

Blank line is not needed, there and below everywhere

4219

There and everywhere

4296

Since you checked the length, what is the point of using strlcpy() instead of strcpy()?

In D38499#876078, @kib wrote:

A lot of functions return int and that boils down to return (0);. Do you intend to make the fail in some way in the future? If yes, could you please describe what are the expected failure modes?

For many of the new ones that return (0); I was just following the precedent. I think the idea was originally (nearly a decade ago) that the netstack module could return an error on an accessor that doesn't exist. For instance, maybe the netstack doesn't support some capability at all, it could return an error if that capability gets set. @stevek knows better of Juniper's intentions then. I'll at least clean up the functions that simply forward to functions that return void.

This revision is now accepted and ready to land.Feb 13 2023, 9:27 PM

Didn't mean to override your comments! I'm pretty sure @jhibbits will address them before checking in.

Address @kib's feedback. I think I got them all.

This revision now requires review to proceed.Feb 14 2023, 1:06 AM

Didn't mean to override your comments! I'm pretty sure @jhibbits will address them before checking in.

I did take care of all of them, but neglected to update the review once I did.

sys/net/if.c
4651
4696

This can be written simpler as this

4708

Is the cast still needed?

4714

Again, the q about the cast.

4729–4730

There should be blank line between decl and body block, and perhaps no blank line before return.

4839

Why the cast is needed?

In fact, I suspect that casting the function pointer through an object pointer is UB, just not sure about a possible special case for void *.

4851

Same

jhibbits added inline comments.
sys/net/if.c
4651

Odd, I thought I got all the naked returns.

4696

I was going with that first, but figured "!=" is a boolean result, so went explicit. I'll drop it.

4839

Ancient code (2014). Cleaning it up.

jhibbits marked 3 inline comments as done.

Address more feedback from @kib.

This revision is now accepted and ready to land.Feb 14 2023, 1:40 AM
This revision was automatically updated to reflect the committed changes.