Page MenuHomeFreeBSD

ifconfig(8): bug: can't use 'name' or 'description' when creating interface with auto numbering
ClosedPublic

Authored by marieheleneka_gmail.com on Feb 19 2016, 3:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 7 2024, 8:31 PM
Unknown Object (File)
Oct 24 2024, 1:43 PM
Unknown Object (File)
Oct 22 2024, 8:10 AM
Unknown Object (File)
Oct 19 2024, 2:28 PM
Unknown Object (File)
Oct 18 2024, 5:29 AM
Unknown Object (File)
Oct 18 2024, 5:29 AM
Unknown Object (File)
Oct 18 2024, 5:29 AM
Unknown Object (File)
Oct 18 2024, 5:29 AM
Subscribers

Details

Summary

PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=206876#c1

This patch fixes the following two bugs:

  • If one does 'ifconfig tap create name blah', it will return error because the 'name' command doesn't properly populate the request sent to ioctl(...). The 'description' command has the same bug, and is also fixed with this patch.
  • If one does 'ifconfig tap create mtu 9000 name blah', it DOES work, but 'tap0' (or other sequence number) is echoed, instead of the expected 'blah'. (assuming the name change actually succeeded)
Test Plan

Some commands to run to test the patch does as intended:

ifconfig tap create
ifconfig tap create name test1
ifconfig tap create description "this is test 2"
ifconfig tap create name test3 description "this is test 3"

Regression tests?

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marieheleneka_gmail.com retitled this revision from to ifconfig(8): bug: can't use 'name' or 'description' when creating interface with auto numbering.
marieheleneka_gmail.com updated this object.
marieheleneka_gmail.com edited the test plan for this revision. (Show Details)
marieheleneka_gmail.com set the repository for this revision to rS FreeBSD src repository - subversion.

Renamed 'echoifname' to 'printifname' to be more consistent.
New behaviour: The 'name' command will now trigger printing of new interface name if the rename is successful. Example: "ifconfig tap0 name blah" will echo "blah" on success, and not echo anything on failure.

Question: Would it be preferred to always echo interface name when the "name" command is used? Assume name 'blah' is already taken, the above command would then print 'tap0'. (plus the error message as it always has, of course.)

Edit: I realize the current revision doesn't provide proper context, will fix after sleep. :)

Updated revision to have proper context

The two strncpy()s are indeed required, that's clearly a bug fix.

I think I agree with your changes to the printing behaviour.

sbin/ifconfig/ifconfig.c
1028 ↗(On Diff #13483)

This should go after the variable declaration.
Also, there should be no space after the sizeof(). (See style(9)).

1049 ↗(On Diff #13483)

This should go after the variable declaration.

"sizeof (...)" vs "sizeof(...)" vs "sizeof ..."

in ifconfig.c, the function setifmetric(...) use "sizeof (ifr.ifr_name)", while the function ifconfig(...) line795 and 812 use "sizeof ifr.ifr_name".

Should I change all these (and similar occurances) to "sizeof(ifr.ifr_name)"?

sbin/ifconfig/ifconfig.c
1028 ↗(On Diff #13483)

Will move it. About sizeof: see separate comment.

1049 ↗(On Diff #13483)

Will move it.

Yeah, ifconfig is horribly inconsistent in that regard. The style(9) is the best reference for what it should be. Let's make sure the new code follows that.

Don't clean up the others in this patch though. That should be done in a separate commit.

marieheleneka_gmail.com edited edge metadata.
marieheleneka_gmail.com marked 4 inline comments as done.

Addressed inline comments.

I think that looks good. I'll commit it in the morning. Thanks!

This revision was automatically updated to reflect the committed changes.