Re: ifconfig ieee80211 scan error to stderr
Le 12/02/11 06:35, Philip Guenther a icrit : On Thu, Dec 1, 2011 at 6:45 PM, Christiano F. Haesbaert wrote: Hi, I think we should warn() on any error, not just EPERM. This is more consistent with the rest of the code. ok ? I disagree with this. The existing message is much clearer to the non-root mortal user that gets it and, IMO, it's clearer for the message to be sent to stdout with the rest of the output from the scan. The original output is an error message sent to stdout. I think the current output is stupid : if I ask about scanning on WIF and permission is denied, I certainly don't want the mac adress and media info to be printed. As for errors other than EPERM, well, exactly what other errors *can* that call return in ifconfig? What problem are you guys trying to solve? Scripting. It's quite usual to have a one liner with interface name and error message, and not bury it inside a bunch of informations about the interface. Now if the rest is sent to stdout it's easy to send it to /dev/null and just get the error message. -- Thomas de Grivel
Re: ifconfig ieee80211 scan error to stderr
On 2 December 2011 03:35, Philip Guenther wrote: > On Thu, Dec 1, 2011 at 6:45 PM, Christiano F. Haesbaert > wrote: >> Hi, I think we should warn() on any error, not just EPERM. >> This is more consistent with the rest of the code. >> >> ok ? > > I disagree with this. The existing message is much clearer to the > non-root mortal user that gets it and, IMO, it's clearer for the > message to be sent to stdout with the rest of the output from the > scan. > > As for errors other than EPERM, well, exactly what other errors *can* > that call return in ifconfig? Now none since the initial block guarantees no ENETDOWN, but when we have another error in there, we'll have a silent error. > > What problem are you guys trying to solve? > Actually just code consistency. But I've wasted too much people's time with this already, it's bikeshed so don't bother.
Re: ifconfig ieee80211 scan error to stderr
> Date: Fri, 2 Dec 2011 00:45:54 -0200
> From: "Christiano F. Haesbaert"
>
> Hi, I think we should warn() on any error, not just EPERM.
> This is more consistent with the rest of the code.
>
> ok ?
I think the current code is just fine. The "no premission to scan"
output is an integral part of the ifconfig output. If you don't have
the right permission, you just don't get to see the information, much
like that you don't get to see the WEP/WPA keys.
Both your and Thomas' change make the output look ugly, and print to
stderr in the middle of stuff printed to stdout. That always annoys
me.
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.252
> diff -d -u -p -b -r1.252 ifconfig.c
> --- ifconfig.c26 Nov 2011 23:38:18 - 1.252
> +++ ifconfig.c2 Dec 2011 02:32:48 -
> @@ -2168,8 +2168,7 @@ ieee80211_listnodes(void)
> strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
>
> if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
> - if (errno == EPERM)
> - printf("\t\tno permission to scan\n");
> + warn("SIOCS80211SCAN");
> goto done;
> }
Re: ifconfig ieee80211 scan error to stderr
On 12/02/2011 12:35 AM, Philip Guenther wrote: On Thu, Dec 1, 2011 at 6:45 PM, Christiano F. Haesbaert wrote: Hi, I think we should warn() on any error, not just EPERM. This is more consistent with the rest of the code. ok ? I disagree with this. The existing message is much clearer to the non-root mortal user that gets it and, IMO, it's clearer for the message to be sent to stdout with the rest of the output from the scan. As for errors other than EPERM, well, exactly what other errors *can* that call return in ifconfig? The existing error message should be retained as Mr. Guenther says. An "else" clause reporting any other error is very desirable if other errors are not anticipated. The general principle of "if an error is reported to you that you don't understand, pass it up the stack until somebody can record it or do something about it" is important. It will save the maintainer's sanity when the kernel or library changes or adds functionality. 99.9% of the reporting code will never be executed. Trade that cost against weeks of frustration. I'd be glad to share gory descriptions of weeks spent chasing unreported errors off line. Geoff Steckel
Re: ifconfig ieee80211 scan error to stderr
On Thu, Dec 1, 2011 at 6:45 PM, Christiano F. Haesbaert wrote: > Hi, I think we should warn() on any error, not just EPERM. > This is more consistent with the rest of the code. > > ok ? I disagree with this. The existing message is much clearer to the non-root mortal user that gets it and, IMO, it's clearer for the message to be sent to stdout with the rest of the output from the scan. As for errors other than EPERM, well, exactly what other errors *can* that call return in ifconfig? What problem are you guys trying to solve? Philip Guenther $ ifconfig rsu0 scan rsu0: flags=8843 mtu 1500 lladdr 00:0a:79:de:84:aa priority: 4 groups: wlan media: IEEE802.11 autoselect status: no network ieee80211: nwid "" no permission to scan $ obj/ifconfig rsu0 scan rsu0: flags=8843 mtu 1500 lladdr 00:0a:79:de:84:aa priority: 4 groups: wlan media: IEEE802.11 autoselect (autoselect mode 11b) status: no network ieee80211: nwid "" ifconfig: SIOCS80211SCAN: Operation not permitted
Re: ifconfig ieee80211 scan error to stderr
Hi, I think we should warn() on any error, not just EPERM.
This is more consistent with the rest of the code.
ok ?
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.252
diff -d -u -p -b -r1.252 ifconfig.c
--- ifconfig.c 26 Nov 2011 23:38:18 - 1.252
+++ ifconfig.c 2 Dec 2011 02:32:48 -
@@ -2168,8 +2168,7 @@ ieee80211_listnodes(void)
strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
- if (errno == EPERM)
- printf("\t\tno permission to scan\n");
+ warn("SIOCS80211SCAN");
goto done;
}
Re: ifconfig ieee80211 scan error to stderr
On 30 November 2011 12:39, Thomas de Grivel wrote:
> Hi,
>
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 ifconfig.c
> --- ifconfig.c26 Nov 2011 23:38:18 -1.252
> +++ ifconfig.c30 Nov 2011 14:35:52 -
> @@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)
>
> if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
> if (errno == EPERM)
> -printf("\t\tno permission to scan\n");
> +fprintf(stderr, "%s: no permission to scan\n", name);
> goto done;
> }
> ? sbin_ifconfig.c-ieee80211-scan-stderr.diff
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 ifconfig.c
> --- ifconfig.c 26 Nov 2011 23:38:18 - 1.252
> +++ ifconfig.c 30 Nov 2011 14:38:25 -
> @@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)
>
>if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
>if (errno == EPERM)
> - printf("\t\tno permission to scan\n");
> + fprintf(stderr, "%s: no permission to scan\n",
name);
>goto done;
>}
>
>
I think this should be warnx() and not fprintf(stderr,
The rest of the code uses warnx().
ifconfig ieee80211 scan error to stderr
Hi,
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.252
diff -u -p -r1.252 ifconfig.c
--- ifconfig.c26 Nov 2011 23:38:18 -1.252
+++ ifconfig.c30 Nov 2011 14:35:52 -
@@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)
if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
if (errno == EPERM)
-printf("\t\tno permission to scan\n");
+fprintf(stderr, "%s: no permission to scan\n", name);
goto done;
}
? sbin_ifconfig.c-ieee80211-scan-stderr.diff
Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.252
diff -u -p -r1.252 ifconfig.c
--- ifconfig.c 26 Nov 2011 23:38:18 - 1.252
+++ ifconfig.c 30 Nov 2011 14:38:25 -
@@ -2169,7 +2169,7 @@ ieee80211_listnodes(void)
if (ioctl(s, SIOCS80211SCAN, (caddr_t)&ifr) != 0) {
if (errno == EPERM)
- printf("\t\tno permission to scan\n");
+ fprintf(stderr, "%s: no permission to scan\n", name);
goto done;
}
