Re: [patch] make ifconfig report 'SIOCSIFMEDIA' when ioctl fails

2017-10-27 Thread Martin Pieuchot
On 26/10/17(Thu) 23:41, Jesper Wallin wrote:
> Hi all,
> 
> First off, as always, I apologize if I'm wasting anyone's time because
> I'm missing something obvious here.
> 
> So, I accidentally ran "ifconfig iwm0 mode 11g" as a regular user and
> noticed it didn't throw an error.  A quick look at the code and it seems
> like the error was left out intentionally.  I consulted stsp@ on IRC
> about it and he explained that the 'media' subcommand will fail when
> running as a regular user if we error out here.  However, after
> re-adding the err(3) and doing some quick testing and trying to
> understand the code some more, things do seem to work?

Running "$ ifconfig em0 media" under ktrace(1) reveals that the only
media ioctl(2) issued is SIOCGIFMEDIA.  So there's no problem there.

> The err(3) was removed in rev 1.203 by deraadt@, about 9 years ago,
> probably because of the reasons stsp@ explained?  Could it be that the
> 'media' part has been rewritten to work even if process_media_commands()
> errors out?
> 
> Anyway, I just thought it's a bit confusing when ifconfig exits
> normally without any notices or errors, even if the command fails.

Diff is correct, ok mpi@

> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.348
> diff -u -p -r1.348 ifconfig.c
> --- ifconfig.c29 Aug 2017 21:10:20 -  1.348
> +++ ifconfig.c26 Oct 2017 19:49:44 -
> @@ -2449,7 +2449,7 @@ process_media_commands(void)
>   ifr.ifr_media = media_current;
>  
>   if (ioctl(s, SIOCSIFMEDIA, (caddr_t)) < 0)
> - ;
> + err(1, "SIOCSIFMEDIA");
>  }
>  
>  /* ARGSUSED */
> 



[patch] make ifconfig report 'SIOCSIFMEDIA' when ioctl fails

2017-10-26 Thread Jesper Wallin
Hi all,

First off, as always, I apologize if I'm wasting anyone's time because
I'm missing something obvious here.

So, I accidentally ran "ifconfig iwm0 mode 11g" as a regular user and
noticed it didn't throw an error.  A quick look at the code and it seems
like the error was left out intentionally.  I consulted stsp@ on IRC
about it and he explained that the 'media' subcommand will fail when
running as a regular user if we error out here.  However, after
re-adding the err(3) and doing some quick testing and trying to
understand the code some more, things do seem to work?

The err(3) was removed in rev 1.203 by deraadt@, about 9 years ago,
probably because of the reasons stsp@ explained?  Could it be that the
'media' part has been rewritten to work even if process_media_commands()
errors out?

Anyway, I just thought it's a bit confusing when ifconfig exits
normally without any notices or errors, even if the command fails.


Jesper Wallin


Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.348
diff -u -p -r1.348 ifconfig.c
--- ifconfig.c  29 Aug 2017 21:10:20 -  1.348
+++ ifconfig.c  26 Oct 2017 19:49:44 -
@@ -2449,7 +2449,7 @@ process_media_commands(void)
ifr.ifr_media = media_current;
 
if (ioctl(s, SIOCSIFMEDIA, (caddr_t)) < 0)
-   ;
+   err(1, "SIOCSIFMEDIA");
 }
 
 /* ARGSUSED */