| Message ID | 1487049663-2942-1-git-send-email-masashi.honma@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
On 2017/02/14 14:21, Masashi Honma wrote: > Previously, bitrate showed uninitialized buffer when no rate info found. > This patch fixes the issue. This is the screen shot. Masashi Honma.
On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote: > Previously, bitrate showed uninitialized buffer when no rate info > found. When would this happen? I'm not really sure this is right - perhaps we don't have RATE_INFO_BITRATE(32), but still have the MCS data? How about we just add "(unknown)" or so and not return here? johannes
On 2017-02-14 17:35, Johannes Berg wrote: > On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote: >> Previously, bitrate showed uninitialized buffer when no rate info >> found. > > When would this happen? I could see in mesh STA connection with 11n and legacy mixed. STA A has disable_ht=1. STA B has disable_ht=0. > I'm not really sure this is right - perhaps we don't have > RATE_INFO_BITRATE(32), but still have the MCS data? I recognized there was a issue on such a case. I will send a patch to wpa_supplicant. Anyway, showing string message is better than showing raw binary data. > How about we just add "(unknown)" or so and not return here? Yes. First time, I supposed to use "unknown". But in the function parse_bitrate(), nla_parse_nested() returns message "failed to parse nested rate attributes!". This explains why the bitrate is unknown. So I used explaining message. We could see the message like this. tx bitrate: No rate info found! rx bitrate: 48.0 MBit/s Masashi Honma.
On Tue, 2017-02-14 at 17:55 +0900, Masashi Honma wrote: > On 2017-02-14 17:35, Johannes Berg wrote: > > On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote: > > > Previously, bitrate showed uninitialized buffer when no rate info > > > found. > > > > When would this happen? > > I could see in mesh STA connection with 11n and legacy mixed. > STA A has disable_ht=1. > STA B has disable_ht=0. Interesting, ok. > > I'm not really sure this is right - perhaps we don't have > > RATE_INFO_BITRATE(32), but still have the MCS data? > > I recognized there was a issue on such a case. I will send a patch > to wpa_supplicant. Anyway, showing string message is better than > showing raw binary data. > > > How about we just add "(unknown)" or so and not return here? > > Yes. First time, I supposed to use "unknown". Ok. > But in the function > parse_bitrate(), nla_parse_nested() returns message "failed to parse > nested rate attributes!". This explains why the bitrate is unknown. Yes, this is bad! But if you saw that, why did you ever get to the below code that checked for the BITRATE(32) attributes? > So I used explaining message. We could see the message like this. > > tx bitrate: No rate info found! > rx bitrate: 48.0 MBit/s Yeah I was just thinking we could also see tx bitrate: (unknown) and then if there was MCS anyway you'd see tx bitrate: (unknown) MCS 7 or something like that? johannes
On 2017-02-14 18:14, Johannes Berg wrote: > Yes, this is bad! But if you saw that, why did you ever get to the > below code that checked for the BITRATE(32) attributes? > >> So I used explaining message. We could see the message like this. >> >> tx bitrate: No rate info found! >> rx bitrate: 48.0 MBit/s > > Yeah I was just thinking we could also see > > tx bitrate: (unknown) > > and then if there was MCS anyway you'd see > > tx bitrate: (unknown) MCS 7 > > or something like that? Yes, showing information as far as possible looks good. I will modify this patch. Masashi Honma.
diff --git a/station.c b/station.c index f3e3da8..9d3eb4d 100644 --- a/station.c +++ b/station.c @@ -151,6 +151,10 @@ void parse_bitrate(struct nlattr *bitrate_attr, char *buf, int buflen) if (rate > 0) pos += snprintf(pos, buflen - (pos - buf), "%d.%d MBit/s", rate / 10, rate % 10); + else { + snprintf(buf, buflen, "No rate info found!"); + return; + } if (rinfo[NL80211_RATE_INFO_MCS]) pos += snprintf(pos, buflen - (pos - buf),
Previously, bitrate showed uninitialized buffer when no rate info found. This patch fixes the issue. Signed-off-by: Masashi Honma <masashi.honma@gmail.com> --- station.c | 4 ++++ 1 file changed, 4 insertions(+)