Re: CVS commit: src/sbin/ifconfig

[Roy Marples <roy%marples.name@localhost>]

On 14/09/2016 13:00, Joerg Sonnenberger wrote:
> On Wed, Sep 14, 2016 at 11:46:43AM +0000, Roy Marples wrote:
>> Module Name:	src
>> Committed By:	roy
>> Date:		Wed Sep 14 11:46:43 UTC 2016
>>
>> Modified Files:
>> 	src/sbin/ifconfig: media.c
>>
>> Log Message:
>> Don't bail if SIOGIFMEDIA doesn't return any media lists because we
>> can still report link status.
> 
> As I mentioned in the review, I think this is the wrong approach. I find
> the assumption that every interface supporting the IFMEDIA interface has
> at least one medium to be perfectly reasonable. Setting up and
> defaulting to one AUTO medium is two lines of code. Now we have to look
> for bugs in every consumer of this interface...

There is no media to select because there is none. Providing a list of
one just to satisfy strikes me as waste of bytes.

Existing code that just cares about link status will function as before
without any change.
Ditto for any code which lists or sets the media in use because there
has to be a list to start with.

The only case where this can possibly fail is where (like ifconfig) the
code assumes that if there is no list then there is no link status -
which is patently wrong itself because we have IFM_AVALID for that.
Old ifconfig still works with the new code, it will just warn that it
can't query a list of media and won't print any link status.

It could also be argued that obtaining link status should be it's own
ioctl and not bolted on-to media.

Roy