Details

Reviewers

kp
allanjude
gnn

Commits

rS296425: MFC r295836:
rS295836: ifconfig(8): can't use 'name' or 'description' when creating interface with…

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 Skipped

Unit

Tests Skipped

Event Timeline

marieheleneka_gmail.com updated this revision to Diff 13471.Feb 19 2016, 3:06 AM

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 added a reviewer: allanjude.

marieheleneka_gmail.com set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptFeb 19 2016, 3:06 AM

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

marieheleneka_gmail.com added reviewers: gnn, kp.Feb 19 2016, 5:13 PM

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.

Addressed inline comments.

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

Closed by commit rS295836: ifconfig(8): can't use 'name' or 'description' when creating interface with… (authored by kp). · Explain WhyFeb 20 2016, 11:37 AM

This revision was automatically updated to reflect the committed changes.

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 13471

ifclone.c

ifconfig.h

ifconfig.c

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 13471

ifclone.c

ifconfig.h

ifconfig.c

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

Revision Contents
Changeset List