Page MenuHomeFreeBSD

ifconfig: Add format shortcuts.
ClosedPublic

Authored by des on May 12 2024, 3:57 PM.
Tags
None
Referenced Files
F88214196: D45166.diff
Fri, Jul 12, 2:27 PM
Unknown Object (File)
May 21 2024, 10:25 AM
Unknown Object (File)
May 19 2024, 12:37 PM
Unknown Object (File)
May 18 2024, 11:40 PM
Unknown Object (File)
May 14 2024, 6:18 PM
Unknown Object (File)
May 14 2024, 7:09 AM
Unknown Object (File)
May 13 2024, 10:53 AM
Unknown Object (File)
May 13 2024, 8:43 AM
Subscribers
None

Diff Detail

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

Event Timeline

des requested review of this revision.May 12 2024, 3:57 PM
des created this revision.
sbin/ifconfig/ifconfig.c
332

Would it be worthwhile to generalize, and allow other formats to be combined across multiple address types? The open question would be which address types to include. One option is to assume "addr". Or maybe it's just better to make "addr:cidr" work, and use that in login.conf. btw, it would also be nice if command-line options overrode the environment; they don't seem to at the moment, but that's a change in behavior.

sbin/ifconfig/ifconfig.c
332

Le mieux est l'ennemi du bien.

sbin/ifconfig/ifconfig.c
332

Perhaps, but inventing a new syntax when there is already one there seems unnecessary. Granted, the "it would be nice" part might fall into that category, but should be a separate change in any case.

des marked 2 inline comments as done.May 13 2024, 10:45 AM
des added inline comments.
sbin/ifconfig/ifconfig.c
332

Command-line options already override the environment variable. The latter is handled on lines 629-631 before the call to args_parse() on line 639.

Are there any other reviewers that may be interested? Maybe post a link on the GitHub review?

sbin/ifconfig/ifconfig.c
332

Oh, I see. I expected -f addr:default to override inet:cidr and inet6:cidr in the environment. Now I see that addr: only applies to the address, whereas inet: and inet6: deal with the netmask/prefix length. The man page could be clearer, e.g. by putting the address formats in a separate list from the mask formats. At any rate, my suggestion of addr:cidr doesn't make sense. This makes "default" a bit irregular, applying to both addresses and masks, but I find the current scheme irregular anyway.

Generally i like it. It does seem to mix memory leak / style fixed in with the new functionality.

Otherwise i think this is a good change. Like mike i could quibble around the edges, but it wouldn't make it that much better..

sbin/ifconfig/ifconfig.c
319

This can be a separate commit

624–625

Why remove this?

This revision is now accepted and ready to land.May 13 2024, 1:59 PM
des marked 3 inline comments as done.May 14 2024, 6:49 AM
des added inline comments.
sbin/ifconfig/ifconfig.c
624–625

It's pointless since these variables are static and therefore already initialized to NULL.

des marked an inline comment as done.May 14 2024, 6:49 AM
des added inline comments.
sbin/ifconfig/ifconfig.c
624–625

Sorry, I meant global, not static.

This revision was automatically updated to reflect the committed changes.