On Tuesday, July 09, 2013 05:22:21 PM Johannes Berg wrote: > > A few nits, since you're going to have to resend anyway (this patch is > also line-wrapped) > > > > NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD > > Is that actually needed at all? Now that we have the "replied" flag in > the first patch, it seems like this wouldn't be used at all? I was thinking about it; and argument to keep this flag is: It is hint for wpa_s, indicating that it may be that frame was answered by card and not reported. Imagine card that answer everything in firmware and don't report any probes at all. Without this hint, wpa_s may mis-interpret this as complete silence. But, to be honest, I am not absolutely sure it is required. If Jouni say he don't need this indication, I'll remove it. Jouni: could you please comment? > > > CMD(crit_proto_stop, CRIT_PROTOCOL_STOP); > > + CMD(start_p2p_find, START_P2P_FIND); > > + CMD(stop_p2p_find, STOP_P2P_FIND); > > That'll probably have to be changed when mac80211 supports this, but we > don't have to worry about it right now. > > > +} > > +static int nl80211_start_p2p_find(struct sk_buff *skb, struct genl_info > > *info) > > There should be a blank line between the two functions :) checkpatch was silent about it > > > + params.channels = kzalloc(n_channels * sizeof(*params.channels), > > + GFP_KERNEL); > > kcalloc? Probably doesn't matter much though. Good point. Why not? > > > + attr = info->attrs[NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL]; > > + if (attr) > > + params.min_discoverable_interval = nla_get_u32(attr); > > + > > + attr = info->attrs[NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL]; > > + if (attr) > > + params.max_discoverable_interval = nla_get_u32(attr); > > No validation at all? What if I pass 7/3 for min/max (yes, in that > order)? Yes, need to validate all data from user space. > > johannes > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
