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)
Mon, Mar 18, 8:07 PM
Unknown Object (File)
Feb 13 2024, 12:57 PM
Unknown Object (File)
Feb 12 2024, 11:43 AM
Unknown Object (File)
Jan 27 2024, 10:35 AM
Unknown Object (File)
Dec 20 2023, 5:52 AM
Unknown Object (File)
Dec 19 2023, 10:26 PM
Unknown Object (File)
Dec 14 2023, 8:22 PM
Unknown Object (File)
Nov 25 2023, 7:50 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 Skipped
Unit
Tests Skipped

Event Timeline

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

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

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

97

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.)

102

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."

105

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
91

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
105

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
97

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.