2015-07-17 1:49 GMT+03:00 Stefan Sperling <s...@stsp.name>:
> On Fri, Jul 17, 2015 at 01:23:54AM +0300, Vadim Zhukov wrote:
>> > + printf(", channels %d", first_chan);
>> > + for (i = first_chan + 1; i < nchan; i++)
>> > + printf(" %i", i);
>>
>> It looks like you simply enumerate from first_chan till nchan; why not
>
> Indeed, this is what the channel data means.
>
>> to just print "channels %d-%d" then? Also, with this for loop, in case
>> of invalid data (nchan < first_chan) you won't have any data printed
>> at all.
>
> Good catch.
>
> My previous diff also didn't properly loop over the channel/tx power list.
>
> New diff. Tested in hackroom, where e.g. on 5GHz I see:
>
> country 'CA ', channels 52-55 max TX power 24dB, channels 132-134 max TX
> power 24dB
Looks nice for me, but two more nits are inline below.
> Index: print-802_11.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/print-802_11.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 print-802_11.c
> --- print-802_11.c 16 Jul 2015 20:57:13 -0000 1.19
> +++ print-802_11.c 16 Jul 2015 22:45:51 -0000
> @@ -78,6 +78,7 @@ int ieee80211_hdr(struct ieee80211_fram
> int ieee80211_data(struct ieee80211_frame *, u_int);
> void ieee80211_print_element(u_int8_t *, u_int);
> void ieee80211_print_essid(u_int8_t *, u_int);
> +void ieee80211_print_country(u_int8_t *, u_int);
> void ieee80211_print_htcaps(u_int8_t *, u_int);
> int ieee80211_elements(struct ieee80211_frame *, u_int);
> int ieee80211_frame(struct ieee80211_frame *, u_int);
> @@ -233,6 +234,43 @@ ieee80211_print_essid(u_int8_t *essid, u
>
> /* Caller checks len */
> void
> +ieee80211_print_country(u_int8_t *data, u_int len)
> +{
> + u_int8_t first_chan, nchan, maxpower;
> +
> + if (len < 6)
Shouldn't this be "len < 3" here?
> + return;
> +
> + /* country string */
> + printf(" '%c%c%c'", data[0], data[1], data[2]);
> +
> + len -= 3;
> + data += 3;
> +
> + /* channels and corresponding TX power limits */
> + while (len > 0) {
> + if (len < 6)
> + break;
> +
> + /* ignore operating extension Id (values >= 201) for now */
> + if (data[3] >= 201 || data[4] >= 201)
> + continue;
> +
> + first_chan = data[3];
> + nchan = data[4];
> + maxpower = data[5];
> +
> + printf(", channel%s %d", nchan > 1 ? "s" : "", first_chan);
> + if (nchan > 1)
> + printf("-%d", first_chan + nchan - 1);
> + printf(" max TX power %ddB", maxpower);
Hm, what if we have received nchan==0, maybe some warning should be
printed? Otherwise, it won't be possible to distinguish from nchan==1
case. I'd suggest checking "nchan != 1" instead of "nchan > 1", then
we'll see strange "channels" part in case of nchan==0, and detect data
error.
> + len -= 6;
> + data += 6;
> + }
> +}
> +/* Caller checks len */
> +void
> ieee80211_print_htcaps(u_int8_t *data, u_int len)
> {
> u_int16_t htcaps;
> @@ -392,8 +430,7 @@ ieee80211_elements(struct ieee80211_fram
> break;
> case IEEE80211_ELEMID_COUNTRY:
> printf(", country");
> - for (i = len; i > 0; i--, data++)
> - printf(" %u", data[0]);
> + ieee80211_print_country(data, len);
> break;
> case IEEE80211_ELEMID_CHALLENGE:
> printf(", challenge");
> @@ -436,6 +473,9 @@ ieee80211_elements(struct ieee80211_fram
> printf(", htcaps");
> if (vflag)
> ieee80211_print_htcaps(data, len);
> + break;
> + case IEEE80211_ELEMID_POWER_CONSTRAINT:
> + printf(", power constraint %udB", data[0]);
> break;
> case IEEE80211_ELEMID_VENDOR:
> printf(", vendor");
--
WBR,
Vadim Zhukov
