[PATCH] cfg80211: change function signature of cfg80211_get_p2p_attr()

[PATCH] cfg80211: change function signature of cfg80211_get_p2p_attr()

[Arend van Spriel]

The function cfg80211_get_p2p_attr() can fail and returns
a negative error code. However, the return type is unsigned
int. The largest positive number is determined by desired_len
variable in the function, which is u16. So changing the return
type to int to allow easy error checking. Also change the type
for the attribute to enum for improved type checking.

Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
Hi Johannes,

I decided to make use of cfg80211_get_p2p_attr() in brcmfmac
and would like this minor change to it.

The only other "driver" using it is mac80211 and it seems it
will not mind this change ;-)

Gr. AvS
---
 include/net/cfg80211.h |    5 +++--
 net/wireless/util.c    |    6 ++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8a1aec5..45eb8ec 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3652,8 +3652,9 @@ void cfg80211_unregister_wdev(struct wireless_dev *wdev);
  * the data is malformed or the attribute can't be found (respectively),
  * or the length of the found attribute (which can be zero).
  */
-unsigned int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
-				   u8 attr, u8 *buf, unsigned int bufsize);
+int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
+			  enum ieee80211_p2p_attr_id attr_id,
+			  u8 *buf, unsigned int bufsize);
 
 /* Logging, debugging and troubleshooting/diagnostic helpers. */
 
diff --git a/net/wireless/util.c b/net/wireless/util.c
index b99f01c..2344077 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -980,9 +980,11 @@ u32 cfg80211_calculate_bitrate(struct rate_info *rate)
 }
 EXPORT_SYMBOL(cfg80211_calculate_bitrate);
 
-unsigned int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
-				   u8 attr, u8 *buf, unsigned int bufsize)
+int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
+				   enum ieee80211_p2p_attr_id attr_id,
+				   u8 *buf, unsigned int bufsize)
 {
+	u8 attr = (u8)attr_id;
 	u8 *out = buf;
 	u16 attr_remaining = 0;
 	bool desired_attr = false;
-- 
1.7.10.4

Re: [PATCH] cfg80211: change function signature of cfg80211_get_p2p_attr()

[Johannes Berg]

On Sun, 2012-11-25 at 19:13 +0100, Arend van Spriel wrote:
> The function cfg80211_get_p2p_attr() can fail and returns
> a negative error code. However, the return type is unsigned
> int. The largest positive number is determined by desired_len
> variable in the function, which is u16. So changing the return
> type to int to allow easy error checking. Also change the type
> for the attribute to enum for improved type checking.
> 
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> ---
> Hi Johannes,
> 
> I decided to make use of cfg80211_get_p2p_attr() in brcmfmac
> and would like this minor change to it.
> 
> The only other "driver" using it is mac80211 and it seems it
> will not mind this change ;-)

Yeah, well, it actually uses an int as this should, thanks for fixing!

Applied, but I've fixed your indentation:

> +int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
> +				   enum ieee80211_p2p_attr_id attr_id,
> +				   u8 *buf, unsigned int bufsize)
>  {
> +	u8 attr = (u8)attr_id;

Also this isn't actually needed, you can compare a u8 to attr later :)

johannes

Re: [PATCH] cfg80211: change function signature of cfg80211_get_p2p_attr()

[Johannes Berg]

On Mon, 2012-11-26 at 11:27 +0100, Johannes Berg wrote:
> On Sun, 2012-11-25 at 19:13 +0100, Arend van Spriel wrote:
> > The function cfg80211_get_p2p_attr() can fail and returns
> > a negative error code. However, the return type is unsigned
> > int. The largest positive number is determined by desired_len
> > variable in the function, which is u16. So changing the return
> > type to int to allow easy error checking. Also change the type
> > for the attribute to enum for improved type checking.
> > 
> > Signed-off-by: Arend van Spriel <arend@broadcom.com>
> > ---
> > Hi Johannes,
> > 
> > I decided to make use of cfg80211_get_p2p_attr() in brcmfmac
> > and would like this minor change to it.
> > 
> > The only other "driver" using it is mac80211 and it seems it
> > will not mind this change ;-)
> 
> Yeah, well, it actually uses an int as this should, thanks for fixing!
> 
> Applied, but I've fixed your indentation:
> 
> > +int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
> > +				   enum ieee80211_p2p_attr_id attr_id,
> > +				   u8 *buf, unsigned int bufsize)
> >  {
> > +	u8 attr = (u8)attr_id;
> 
> Also this isn't actually needed, you can compare a u8 to attr later :)

And if we change it back, this patch also doesn't break
kernel-doc ... :P

johannes

Re: [PATCH] cfg80211: change function signature of cfg80211_get_p2p_attr()

[Arend van Spriel]

On 11/26/2012 11:29 AM, Johannes Berg wrote:
> On Mon, 2012-11-26 at 11:27 +0100, Johannes Berg wrote:
>> On Sun, 2012-11-25 at 19:13 +0100, Arend van Spriel wrote:
>>> The function cfg80211_get_p2p_attr() can fail and returns
>>> a negative error code. However, the return type is unsigned
>>> int. The largest positive number is determined by desired_len
>>> variable in the function, which is u16. So changing the return
>>> type to int to allow easy error checking. Also change the type
>>> for the attribute to enum for improved type checking.
>>>
>>> Signed-off-by: Arend van Spriel <arend@broadcom.com>
>>> ---
>>> Hi Johannes,
>>>
>>> I decided to make use of cfg80211_get_p2p_attr() in brcmfmac
>>> and would like this minor change to it.
>>>
>>> The only other "driver" using it is mac80211 and it seems it
>>> will not mind this change ;-)
>>
>> Yeah, well, it actually uses an int as this should, thanks for fixing!
>>
>> Applied, but I've fixed your indentation:
>>
>>> +int cfg80211_get_p2p_attr(const u8 *ies, unsigned int len,
>>> +				   enum ieee80211_p2p_attr_id attr_id,
>>> +				   u8 *buf, unsigned int bufsize)
>>>  {
>>> +	u8 attr = (u8)attr_id;
>>
>> Also this isn't actually needed, you can compare a u8 to attr later :)
> 
> And if we change it back, this patch also doesn't break
> kernel-doc ... :P

Overlooked that one. I should add a 'generate kernel-doc' step to my
patch checklist. Thanks for taking care of it.

Gr. AvS