Page MenuHomeFreeBSD

sbin/ifconfig: Use libifconfig to get groups
ClosedPublic

Authored by freqlabs on Feb 27 2021, 8:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 14, 4:17 AM
Unknown Object (File)
Sun, Dec 8, 1:33 AM
Unknown Object (File)
Thu, Dec 5, 2:04 AM
Unknown Object (File)
Sat, Nov 30, 10:51 AM
Unknown Object (File)
Thu, Nov 28, 7:13 PM
Unknown Object (File)
Wed, Nov 27, 4:29 PM
Unknown Object (File)
Nov 22 2024, 11:42 AM
Unknown Object (File)
Nov 14 2024, 8:55 PM
Subscribers
None

Details

Summary

More code deduplication.

Test Plan
# ifconfig > /tmp/a
# git stash pop && make -C sbin/ifconfig && make -C sbin/ifconfig install
<snip>
# ifconfig > /tmp/b
# cmp /tmp/a /tmp/b && echo ok
ok

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

freqlabs created this revision.
freqlabs added a reviewer: kp.
sbin/ifconfig/ifgroup.c
90

Does that shadow the global name variable that we used to use as input?

I don't understand how this is intended to work.

93–95

Are we changing the input from the name global to the ifr.ifr_name global?

(We really need to clean up the use of globals in ifconfig, it's hard to follow for the small of brain, like me.)

104

That needs to be at the top of the block.

style(9) still has "Declarations may be placed before executable lines at the start of any block."

107

Does the kernel not set .ifgr_len to the correct size for the number of groups it returned?

If not, is that something we should have ifconfig_get_groups() clean up for us? It seems silly for all users of the lib to have to worry about this.

sbin/ifconfig/ifgroup.c
90

Yes I started to catch on to that after getting into a few more of these. I'll remove the local name variables so the global is used for now, and eventually it can be made a parameter to the function along with the handle.

sbin/ifconfig/ifgroup.c
107

Now that I think about it some more, this check is nonsense. The NULL check in the original code here didn't make sense. We were incrementing and checking a pointer we allocated and already checked for NULL ourselves. We're iterating through an array of structs, not an array of pointers.

I'll remove it.

freqlabs added inline comments.
sbin/ifconfig/ifgroup.c
93–95

I agree, I'm hoping to minimize the use of global variables in the long run.

This revision is now accepted and ready to land.Feb 28 2021, 3:59 PM
This revision was automatically updated to reflect the committed changes.