All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
@ 2015-10-22 10:11 Shivani Bhardwaj
  2015-10-22 10:34 ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 10:11 UTC (permalink / raw)
  To: outreachy-kernel

Remove the wrapper functions get_address1, get_address2 and get_address3
as they are not required and replace their calls by suitable functions.
Semantic patch used:
@@
identifier f,g;
@@

*f(...) {
 g(...); }

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index 427adfd..38bc2c4 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
 	return ((header[1] & 0x02) >> 1);
 }
 
-/* This function extracts the MAC Address in 'address1' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address1(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 4, 6);
-}
-
-/* This function extracts the MAC Address in 'address2' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address2(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 10, 6);
-}
-
-/* This function extracts the MAC Address in 'address3' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address3(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 16, 6);
-}
-
 /* This function extracts the BSSID from the incoming WLAN packet based on   */
-/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
+/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
 /* variable.                                                                 */
 static inline void get_BSSID(u8 *data, u8 *bssid)
 {
 	if (get_from_ds(data) == 1)
-		get_address2(data, bssid);
+		/* Extract the MAC Address in 'address2' field of the MAC */
+		/* header and update the MAC Address in the allocated 'data'
+		 * variable. */
+		memcpy(data, bssid + 10, 6);
 	else if (get_to_ds(data) == 1)
-		get_address1(data, bssid);
+		/* Extract the MAC Address in 'address1' field of the MAC */
+		/* header and update the MAC Address in the allocated 'data'
+		 * variable. */
+		memcpy(data, bssid + 4, 6);
 	else
-		get_address3(data, bssid);
+		/* Extract the MAC Address in 'address3' field of the MAC */
+		/* header and update the MAC Address in the allocated 'data'
+		 * variable. */
+		memcpy(data, bssid + 16, 6);
 }
 
 /* This function extracts the SSID from a beacon/probe response frame        */
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 10:11 [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions Shivani Bhardwaj
@ 2015-10-22 10:34 ` Julia Lawall
  2015-10-22 11:01   ` Shivani Bhardwaj
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-10-22 10:34 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel

On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:

> Remove the wrapper functions get_address1, get_address2 and get_address3
> as they are not required and replace their calls by suitable functions.

The existing functions serve two purposes:

1.  They make clear that an ethernet address is being copied
2.  They hide the offsets

To preserve the first property, the kernel has a function for copying
ethernet addresses.

To preserve the second property, maybe it would be possible to #define
constants that describe the offsets, if there is any way to figure out
what they semantically mean.

julia

> Semantic patch used:
> @@
> identifier f,g;
> @@
>
> *f(...) {
>  g(...); }
>
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> ---
>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
>  1 file changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> index 427adfd..38bc2c4 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.c
> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
>  	return ((header[1] & 0x02) >> 1);
>  }
>
> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> -static inline void get_address1(u8 *pu8msa, u8 *addr)
> -{
> -	memcpy(addr, pu8msa + 4, 6);
> -}
> -
> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> -static inline void get_address2(u8 *pu8msa, u8 *addr)
> -{
> -	memcpy(addr, pu8msa + 10, 6);
> -}
> -
> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> -static inline void get_address3(u8 *pu8msa, u8 *addr)
> -{
> -	memcpy(addr, pu8msa + 16, 6);
> -}
> -
>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
>  /* variable.                                                                 */
>  static inline void get_BSSID(u8 *data, u8 *bssid)
>  {
>  	if (get_from_ds(data) == 1)
> -		get_address2(data, bssid);
> +		/* Extract the MAC Address in 'address2' field of the MAC */
> +		/* header and update the MAC Address in the allocated 'data'
> +		 * variable. */
> +		memcpy(data, bssid + 10, 6);
>  	else if (get_to_ds(data) == 1)
> -		get_address1(data, bssid);
> +		/* Extract the MAC Address in 'address1' field of the MAC */
> +		/* header and update the MAC Address in the allocated 'data'
> +		 * variable. */
> +		memcpy(data, bssid + 4, 6);
>  	else
> -		get_address3(data, bssid);
> +		/* Extract the MAC Address in 'address3' field of the MAC */
> +		/* header and update the MAC Address in the allocated 'data'
> +		 * variable. */
> +		memcpy(data, bssid + 16, 6);
>  }
>
>  /* This function extracts the SSID from a beacon/probe response frame        */
> --
> 2.1.0
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 10:34 ` [Outreachy kernel] " Julia Lawall
@ 2015-10-22 11:01   ` Shivani Bhardwaj
  2015-10-22 11:13     ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 11:01 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>
>> Remove the wrapper functions get_address1, get_address2 and get_address3
>> as they are not required and replace their calls by suitable functions.
>
> The existing functions serve two purposes:
>
> 1.  They make clear that an ethernet address is being copied
> 2.  They hide the offsets
>
> To preserve the first property, the kernel has a function for copying
> ethernet addresses.
>
> To preserve the second property, maybe it would be possible to #define
> constants that describe the offsets, if there is any way to figure out
> what they semantically mean.
>
> julia
>

Right. I am thinking of writing one wrapper function instead of three
where I'll pass on an offset each time which will be #defined.
For example,

#define ofs 4
#define c 6

static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
        ether_addr_copy(addr, pum8sa + offset);
}

func()
{
         if(...)
             get_address(data, bssid, ofs);               //for address1
         else if(...)
             get_address(data, bssid, ofs + c);          //for address2
         else
             get_address(data, bssid, ofs + 2*c);              //for address3
}

This will make the work of function clear. Also, offset is not going
to be clear.
Is this going to be a good idea?

Thank you

>> Semantic patch used:
>> @@
>> identifier f,g;
>> @@
>>
>> *f(...) {
>>  g(...); }
>>
>> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> ---
>>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
>>  1 file changed, 13 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
>> index 427adfd..38bc2c4 100644
>> --- a/drivers/staging/wilc1000/coreconfigurator.c
>> +++ b/drivers/staging/wilc1000/coreconfigurator.c
>> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
>>       return ((header[1] & 0x02) >> 1);
>>  }
>>
>> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
>> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> -static inline void get_address1(u8 *pu8msa, u8 *addr)
>> -{
>> -     memcpy(addr, pu8msa + 4, 6);
>> -}
>> -
>> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
>> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> -static inline void get_address2(u8 *pu8msa, u8 *addr)
>> -{
>> -     memcpy(addr, pu8msa + 10, 6);
>> -}
>> -
>> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
>> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> -static inline void get_address3(u8 *pu8msa, u8 *addr)
>> -{
>> -     memcpy(addr, pu8msa + 16, 6);
>> -}
>> -
>>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
>> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
>> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
>>  /* variable.                                                                 */
>>  static inline void get_BSSID(u8 *data, u8 *bssid)
>>  {
>>       if (get_from_ds(data) == 1)
>> -             get_address2(data, bssid);
>> +             /* Extract the MAC Address in 'address2' field of the MAC */
>> +             /* header and update the MAC Address in the allocated 'data'
>> +              * variable. */
>> +             memcpy(data, bssid + 10, 6);
>>       else if (get_to_ds(data) == 1)
>> -             get_address1(data, bssid);
>> +             /* Extract the MAC Address in 'address1' field of the MAC */
>> +             /* header and update the MAC Address in the allocated 'data'
>> +              * variable. */
>> +             memcpy(data, bssid + 4, 6);
>>       else
>> -             get_address3(data, bssid);
>> +             /* Extract the MAC Address in 'address3' field of the MAC */
>> +             /* header and update the MAC Address in the allocated 'data'
>> +              * variable. */
>> +             memcpy(data, bssid + 16, 6);
>>  }
>>
>>  /* This function extracts the SSID from a beacon/probe response frame        */
>> --
>> 2.1.0
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
>> For more options, visit https://groups.google.com/d/optout.
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 11:01   ` Shivani Bhardwaj
@ 2015-10-22 11:13     ` Julia Lawall
  2015-10-22 11:28       ` Shivani Bhardwaj
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-10-22 11:13 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel

On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:

> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> Remove the wrapper functions get_address1, get_address2 and get_address3
> >> as they are not required and replace their calls by suitable functions.
> >
> > The existing functions serve two purposes:
> >
> > 1.  They make clear that an ethernet address is being copied
> > 2.  They hide the offsets
> >
> > To preserve the first property, the kernel has a function for copying
> > ethernet addresses.
> >
> > To preserve the second property, maybe it would be possible to #define
> > constants that describe the offsets, if there is any way to figure out
> > what they semantically mean.
> >
> > julia
> >
>
> Right. I am thinking of writing one wrapper function instead of three
> where I'll pass on an offset each time which will be #defined.
> For example,
>
> #define ofs 4
> #define c 6

#define things should always be in all (or at least mostly) capital
letters.

> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
>         ether_addr_copy(addr, pum8sa + offset);
> }

I don't think this is the best idea.  ether_addr_copy is something that is
easily understandable to someone reading the code.  get_address, on the
other hand, could be anything.  If you can find meaningful names for the
offsets, then it would be better to say eg:

ether_addr_copy(addr, pum8sa + NAME1)

julia

> func()
> {
>          if(...)
>              get_address(data, bssid, ofs);               //for address1
>          else if(...)
>              get_address(data, bssid, ofs + c);          //for address2
>          else
>              get_address(data, bssid, ofs + 2*c);              //for address3
> }
>
> This will make the work of function clear. Also, offset is not going
> to be clear.
> Is this going to be a good idea?
>
> Thank you
>
> >> Semantic patch used:
> >> @@
> >> identifier f,g;
> >> @@
> >>
> >> *f(...) {
> >>  g(...); }
> >>
> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> >> ---
> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
> >>  1 file changed, 13 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> >> index 427adfd..38bc2c4 100644
> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
> >>       return ((header[1] & 0x02) >> 1);
> >>  }
> >>
> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
> >> -{
> >> -     memcpy(addr, pu8msa + 4, 6);
> >> -}
> >> -
> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
> >> -{
> >> -     memcpy(addr, pu8msa + 10, 6);
> >> -}
> >> -
> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
> >> -{
> >> -     memcpy(addr, pu8msa + 16, 6);
> >> -}
> >> -
> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
> >>  /* variable.                                                                 */
> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
> >>  {
> >>       if (get_from_ds(data) == 1)
> >> -             get_address2(data, bssid);
> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
> >> +             /* header and update the MAC Address in the allocated 'data'
> >> +              * variable. */
> >> +             memcpy(data, bssid + 10, 6);
> >>       else if (get_to_ds(data) == 1)
> >> -             get_address1(data, bssid);
> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
> >> +             /* header and update the MAC Address in the allocated 'data'
> >> +              * variable. */
> >> +             memcpy(data, bssid + 4, 6);
> >>       else
> >> -             get_address3(data, bssid);
> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
> >> +             /* header and update the MAC Address in the allocated 'data'
> >> +              * variable. */
> >> +             memcpy(data, bssid + 16, 6);
> >>  }
> >>
> >>  /* This function extracts the SSID from a beacon/probe response frame        */
> >> --
> >> 2.1.0
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 11:13     ` Julia Lawall
@ 2015-10-22 11:28       ` Shivani Bhardwaj
  2015-10-22 11:43         ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 11:28 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>
>> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >
>> >> Remove the wrapper functions get_address1, get_address2 and get_address3
>> >> as they are not required and replace their calls by suitable functions.
>> >
>> > The existing functions serve two purposes:
>> >
>> > 1.  They make clear that an ethernet address is being copied
>> > 2.  They hide the offsets
>> >
>> > To preserve the first property, the kernel has a function for copying
>> > ethernet addresses.
>> >
>> > To preserve the second property, maybe it would be possible to #define
>> > constants that describe the offsets, if there is any way to figure out
>> > what they semantically mean.
>> >
>> > julia
>> >
>>
>> Right. I am thinking of writing one wrapper function instead of three
>> where I'll pass on an offset each time which will be #defined.
>> For example,
>>
>> #define ofs 4
>> #define c 6
>
> #define things should always be in all (or at least mostly) capital
> letters.
>
>> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
>>         ether_addr_copy(addr, pum8sa + offset);
>> }
>
> I don't think this is the best idea.  ether_addr_copy is something that is
> easily understandable to someone reading the code.  get_address, on the
> other hand, could be anything.  If you can find meaningful names for the
> offsets, then it would be better to say eg:
>
> ether_addr_copy(addr, pum8sa + NAME1)
>
> julia
>

#define NAME1 4
#define NAME2 10
#define NAME3 16

func()
{
         if(...)
            ether_addr_copy(addr, pum8sa + NAME1);               //for address1
         else if(...)
             ether_addr_copy(addr, pum8sa + NAME2));          //for address2
         else
             ether_addr_copy(addr, pum8sa + NAME3);              //for address3
}

Like this?

>> func()
>> {
>>          if(...)
>>              get_address(data, bssid, ofs);               //for address1
>>          else if(...)
>>              get_address(data, bssid, ofs + c);          //for address2
>>          else
>>              get_address(data, bssid, ofs + 2*c);              //for address3
>> }
>>
>> This will make the work of function clear. Also, offset is not going
>> to be clear.
>> Is this going to be a good idea?
>>
>> Thank you
>>
>> >> Semantic patch used:
>> >> @@
>> >> identifier f,g;
>> >> @@
>> >>
>> >> *f(...) {
>> >>  g(...); }
>> >>
>> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> >> ---
>> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
>> >>  1 file changed, 13 insertions(+), 25 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
>> >> index 427adfd..38bc2c4 100644
>> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
>> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
>> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
>> >>       return ((header[1] & 0x02) >> 1);
>> >>  }
>> >>
>> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
>> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
>> >> -{
>> >> -     memcpy(addr, pu8msa + 4, 6);
>> >> -}
>> >> -
>> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
>> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
>> >> -{
>> >> -     memcpy(addr, pu8msa + 10, 6);
>> >> -}
>> >> -
>> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
>> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
>> >> -{
>> >> -     memcpy(addr, pu8msa + 16, 6);
>> >> -}
>> >> -
>> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
>> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
>> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
>> >>  /* variable.                                                                 */
>> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
>> >>  {
>> >>       if (get_from_ds(data) == 1)
>> >> -             get_address2(data, bssid);
>> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
>> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> +              * variable. */
>> >> +             memcpy(data, bssid + 10, 6);
>> >>       else if (get_to_ds(data) == 1)
>> >> -             get_address1(data, bssid);
>> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
>> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> +              * variable. */
>> >> +             memcpy(data, bssid + 4, 6);
>> >>       else
>> >> -             get_address3(data, bssid);
>> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
>> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> +              * variable. */
>> >> +             memcpy(data, bssid + 16, 6);
>> >>  }
>> >>
>> >>  /* This function extracts the SSID from a beacon/probe response frame        */
>> >> --
>> >> 2.1.0
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 11:28       ` Shivani Bhardwaj
@ 2015-10-22 11:43         ` Julia Lawall
  2015-10-22 12:26           ` Shivani Bhardwaj
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-10-22 11:43 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:

> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >
> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
> >> >> as they are not required and replace their calls by suitable functions.
> >> >
> >> > The existing functions serve two purposes:
> >> >
> >> > 1.  They make clear that an ethernet address is being copied
> >> > 2.  They hide the offsets
> >> >
> >> > To preserve the first property, the kernel has a function for copying
> >> > ethernet addresses.
> >> >
> >> > To preserve the second property, maybe it would be possible to #define
> >> > constants that describe the offsets, if there is any way to figure out
> >> > what they semantically mean.
> >> >
> >> > julia
> >> >
> >>
> >> Right. I am thinking of writing one wrapper function instead of three
> >> where I'll pass on an offset each time which will be #defined.
> >> For example,
> >>
> >> #define ofs 4
> >> #define c 6
> >
> > #define things should always be in all (or at least mostly) capital
> > letters.
> >
> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
> >>         ether_addr_copy(addr, pum8sa + offset);
> >> }
> >
> > I don't think this is the best idea.  ether_addr_copy is something that is
> > easily understandable to someone reading the code.  get_address, on the
> > other hand, could be anything.  If you can find meaningful names for the
> > offsets, then it would be better to say eg:
> >
> > ether_addr_copy(addr, pum8sa + NAME1)
> >
> > julia
> >
>
> #define NAME1 4
> #define NAME2 10
> #define NAME3 16

Sorry, NAME was just supposed to be a metavariable, that you could fill in
with something appropriate.  From the comments in the code it seems like
it should be ADDRESS1, ADDRESS2, etc.

julia


>
> func()
> {
>          if(...)
>             ether_addr_copy(addr, pum8sa + NAME1);               //for address1
>          else if(...)
>              ether_addr_copy(addr, pum8sa + NAME2));          //for address2
>          else
>              ether_addr_copy(addr, pum8sa + NAME3);              //for address3
> }
>
> Like this?
>
> >> func()
> >> {
> >>          if(...)
> >>              get_address(data, bssid, ofs);               //for address1
> >>          else if(...)
> >>              get_address(data, bssid, ofs + c);          //for address2
> >>          else
> >>              get_address(data, bssid, ofs + 2*c);              //for address3
> >> }
> >>
> >> This will make the work of function clear. Also, offset is not going
> >> to be clear.
> >> Is this going to be a good idea?
> >>
> >> Thank you
> >>
> >> >> Semantic patch used:
> >> >> @@
> >> >> identifier f,g;
> >> >> @@
> >> >>
> >> >> *f(...) {
> >> >>  g(...); }
> >> >>
> >> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> >> >> ---
> >> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
> >> >>  1 file changed, 13 insertions(+), 25 deletions(-)
> >> >>
> >> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> >> >> index 427adfd..38bc2c4 100644
> >> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
> >> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> >> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
> >> >>       return ((header[1] & 0x02) >> 1);
> >> >>  }
> >> >>
> >> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
> >> >> -{
> >> >> -     memcpy(addr, pu8msa + 4, 6);
> >> >> -}
> >> >> -
> >> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
> >> >> -{
> >> >> -     memcpy(addr, pu8msa + 10, 6);
> >> >> -}
> >> >> -
> >> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
> >> >> -{
> >> >> -     memcpy(addr, pu8msa + 16, 6);
> >> >> -}
> >> >> -
> >> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
> >> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
> >> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
> >> >>  /* variable.                                                                 */
> >> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
> >> >>  {
> >> >>       if (get_from_ds(data) == 1)
> >> >> -             get_address2(data, bssid);
> >> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> +              * variable. */
> >> >> +             memcpy(data, bssid + 10, 6);
> >> >>       else if (get_to_ds(data) == 1)
> >> >> -             get_address1(data, bssid);
> >> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> +              * variable. */
> >> >> +             memcpy(data, bssid + 4, 6);
> >> >>       else
> >> >> -             get_address3(data, bssid);
> >> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> +              * variable. */
> >> >> +             memcpy(data, bssid + 16, 6);
> >> >>  }
> >> >>
> >> >>  /* This function extracts the SSID from a beacon/probe response frame        */
> >> >> --
> >> >> 2.1.0
> >> >>
> >> >> --
> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
> >> >> For more options, visit https://groups.google.com/d/optout.
> >> >>
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQHP7dpLZdz6E%3DCC%2BziN4df1Cdtb7Es-qc2KCFVnXrpF6w%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 11:43         ` Julia Lawall
@ 2015-10-22 12:26           ` Shivani Bhardwaj
  2015-10-22 12:30             ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 12:26 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Thu, Oct 22, 2015 at 5:13 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>
>> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >
>> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >
>> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
>> >> >> as they are not required and replace their calls by suitable functions.
>> >> >
>> >> > The existing functions serve two purposes:
>> >> >
>> >> > 1.  They make clear that an ethernet address is being copied
>> >> > 2.  They hide the offsets
>> >> >
>> >> > To preserve the first property, the kernel has a function for copying
>> >> > ethernet addresses.
>> >> >
>> >> > To preserve the second property, maybe it would be possible to #define
>> >> > constants that describe the offsets, if there is any way to figure out
>> >> > what they semantically mean.
>> >> >
>> >> > julia
>> >> >
>> >>
>> >> Right. I am thinking of writing one wrapper function instead of three
>> >> where I'll pass on an offset each time which will be #defined.
>> >> For example,
>> >>
>> >> #define ofs 4
>> >> #define c 6
>> >
>> > #define things should always be in all (or at least mostly) capital
>> > letters.
>> >
>> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
>> >>         ether_addr_copy(addr, pum8sa + offset);
>> >> }
>> >
>> > I don't think this is the best idea.  ether_addr_copy is something that is
>> > easily understandable to someone reading the code.  get_address, on the
>> > other hand, could be anything.  If you can find meaningful names for the
>> > offsets, then it would be better to say eg:
>> >
>> > ether_addr_copy(addr, pum8sa + NAME1)
>> >
>> > julia
>> >
>>
>> #define NAME1 4
>> #define NAME2 10
>> #define NAME3 16
>
> Sorry, NAME was just supposed to be a metavariable, that you could fill in
> with something appropriate.  From the comments in the code it seems like
> it should be ADDRESS1, ADDRESS2, etc.
>
> julia
>
>

Yes, I understand that. I was just presenting a prototype. :)
I was thinking of ADDR1, ADDR2.. But, ADDRESS1, ADDRESS2,.. would be fine too.

Thank you, Julia! :)

>>
>> func()
>> {
>>          if(...)
>>             ether_addr_copy(addr, pum8sa + NAME1);               //for address1
>>          else if(...)
>>              ether_addr_copy(addr, pum8sa + NAME2));          //for address2
>>          else
>>              ether_addr_copy(addr, pum8sa + NAME3);              //for address3
>> }
>>
>> Like this?
>>
>> >> func()
>> >> {
>> >>          if(...)
>> >>              get_address(data, bssid, ofs);               //for address1
>> >>          else if(...)
>> >>              get_address(data, bssid, ofs + c);          //for address2
>> >>          else
>> >>              get_address(data, bssid, ofs + 2*c);              //for address3
>> >> }
>> >>
>> >> This will make the work of function clear. Also, offset is not going
>> >> to be clear.
>> >> Is this going to be a good idea?
>> >>
>> >> Thank you
>> >>
>> >> >> Semantic patch used:
>> >> >> @@
>> >> >> identifier f,g;
>> >> >> @@
>> >> >>
>> >> >> *f(...) {
>> >> >>  g(...); }
>> >> >>
>> >> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> >> >> ---
>> >> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
>> >> >>  1 file changed, 13 insertions(+), 25 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> index 427adfd..38bc2c4 100644
>> >> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
>> >> >>       return ((header[1] & 0x02) >> 1);
>> >> >>  }
>> >> >>
>> >> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
>> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
>> >> >> -{
>> >> >> -     memcpy(addr, pu8msa + 4, 6);
>> >> >> -}
>> >> >> -
>> >> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
>> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
>> >> >> -{
>> >> >> -     memcpy(addr, pu8msa + 10, 6);
>> >> >> -}
>> >> >> -
>> >> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
>> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
>> >> >> -{
>> >> >> -     memcpy(addr, pu8msa + 16, 6);
>> >> >> -}
>> >> >> -
>> >> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
>> >> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
>> >> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
>> >> >>  /* variable.                                                                 */
>> >> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
>> >> >>  {
>> >> >>       if (get_from_ds(data) == 1)
>> >> >> -             get_address2(data, bssid);
>> >> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
>> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> +              * variable. */
>> >> >> +             memcpy(data, bssid + 10, 6);
>> >> >>       else if (get_to_ds(data) == 1)
>> >> >> -             get_address1(data, bssid);
>> >> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
>> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> +              * variable. */
>> >> >> +             memcpy(data, bssid + 4, 6);
>> >> >>       else
>> >> >> -             get_address3(data, bssid);
>> >> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
>> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> +              * variable. */
>> >> >> +             memcpy(data, bssid + 16, 6);
>> >> >>  }
>> >> >>
>> >> >>  /* This function extracts the SSID from a beacon/probe response frame        */
>> >> >> --
>> >> >> 2.1.0
>> >> >>
>> >> >> --
>> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
>> >> >> For more options, visit https://groups.google.com/d/optout.
>> >> >>
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQHP7dpLZdz6E%3DCC%2BziN4df1Cdtb7Es-qc2KCFVnXrpF6w%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 12:26           ` Shivani Bhardwaj
@ 2015-10-22 12:30             ` Julia Lawall
  2015-10-22 12:33               ` Shivani Bhardwaj
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-10-22 12:30 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:

> On Thu, Oct 22, 2015 at 5:13 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >
> >> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >> >
> >> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
> >> >> >> as they are not required and replace their calls by suitable functions.
> >> >> >
> >> >> > The existing functions serve two purposes:
> >> >> >
> >> >> > 1.  They make clear that an ethernet address is being copied
> >> >> > 2.  They hide the offsets
> >> >> >
> >> >> > To preserve the first property, the kernel has a function for copying
> >> >> > ethernet addresses.
> >> >> >
> >> >> > To preserve the second property, maybe it would be possible to #define
> >> >> > constants that describe the offsets, if there is any way to figure out
> >> >> > what they semantically mean.
> >> >> >
> >> >> > julia
> >> >> >
> >> >>
> >> >> Right. I am thinking of writing one wrapper function instead of three
> >> >> where I'll pass on an offset each time which will be #defined.
> >> >> For example,
> >> >>
> >> >> #define ofs 4
> >> >> #define c 6
> >> >
> >> > #define things should always be in all (or at least mostly) capital
> >> > letters.
> >> >
> >> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
> >> >>         ether_addr_copy(addr, pum8sa + offset);
> >> >> }
> >> >
> >> > I don't think this is the best idea.  ether_addr_copy is something that is
> >> > easily understandable to someone reading the code.  get_address, on the
> >> > other hand, could be anything.  If you can find meaningful names for the
> >> > offsets, then it would be better to say eg:
> >> >
> >> > ether_addr_copy(addr, pum8sa + NAME1)
> >> >
> >> > julia
> >> >
> >>
> >> #define NAME1 4
> >> #define NAME2 10
> >> #define NAME3 16
> >
> > Sorry, NAME was just supposed to be a metavariable, that you could fill in
> > with something appropriate.  From the comments in the code it seems like
> > it should be ADDRESS1, ADDRESS2, etc.
> >
> > julia
> >
> >
>
> Yes, I understand that. I was just presenting a prototype. :)
> I was thinking of ADDR1, ADDR2.. But, ADDRESS1, ADDRESS2,.. would be fine too.

Either way should be OK.

julia

>
> Thank you, Julia! :)
>
> >>
> >> func()
> >> {
> >>          if(...)
> >>             ether_addr_copy(addr, pum8sa + NAME1);               //for address1
> >>          else if(...)
> >>              ether_addr_copy(addr, pum8sa + NAME2));          //for address2
> >>          else
> >>              ether_addr_copy(addr, pum8sa + NAME3);              //for address3
> >> }
> >>
> >> Like this?
> >>
> >> >> func()
> >> >> {
> >> >>          if(...)
> >> >>              get_address(data, bssid, ofs);               //for address1
> >> >>          else if(...)
> >> >>              get_address(data, bssid, ofs + c);          //for address2
> >> >>          else
> >> >>              get_address(data, bssid, ofs + 2*c);              //for address3
> >> >> }
> >> >>
> >> >> This will make the work of function clear. Also, offset is not going
> >> >> to be clear.
> >> >> Is this going to be a good idea?
> >> >>
> >> >> Thank you
> >> >>
> >> >> >> Semantic patch used:
> >> >> >> @@
> >> >> >> identifier f,g;
> >> >> >> @@
> >> >> >>
> >> >> >> *f(...) {
> >> >> >>  g(...); }
> >> >> >>
> >> >> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> >> >> >> ---
> >> >> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
> >> >> >>  1 file changed, 13 insertions(+), 25 deletions(-)
> >> >> >>
> >> >> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
> >> >> >> index 427adfd..38bc2c4 100644
> >> >> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
> >> >> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
> >> >> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
> >> >> >>       return ((header[1] & 0x02) >> 1);
> >> >> >>  }
> >> >> >>
> >> >> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
> >> >> >> -{
> >> >> >> -     memcpy(addr, pu8msa + 4, 6);
> >> >> >> -}
> >> >> >> -
> >> >> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
> >> >> >> -{
> >> >> >> -     memcpy(addr, pu8msa + 10, 6);
> >> >> >> -}
> >> >> >> -
> >> >> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
> >> >> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
> >> >> >> -{
> >> >> >> -     memcpy(addr, pu8msa + 16, 6);
> >> >> >> -}
> >> >> >> -
> >> >> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
> >> >> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
> >> >> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
> >> >> >>  /* variable.                                                                 */
> >> >> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
> >> >> >>  {
> >> >> >>       if (get_from_ds(data) == 1)
> >> >> >> -             get_address2(data, bssid);
> >> >> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> >> +              * variable. */
> >> >> >> +             memcpy(data, bssid + 10, 6);
> >> >> >>       else if (get_to_ds(data) == 1)
> >> >> >> -             get_address1(data, bssid);
> >> >> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> >> +              * variable. */
> >> >> >> +             memcpy(data, bssid + 4, 6);
> >> >> >>       else
> >> >> >> -             get_address3(data, bssid);
> >> >> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
> >> >> >> +              * variable. */
> >> >> >> +             memcpy(data, bssid + 16, 6);
> >> >> >>  }
> >> >> >>
> >> >> >>  /* This function extracts the SSID from a beacon/probe response frame        */
> >> >> >> --
> >> >> >> 2.1.0
> >> >> >>
> >> >> >> --
> >> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> >> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
> >> >> >> For more options, visit https://groups.google.com/d/optout.
> >> >> >>
> >> >>
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQHP7dpLZdz6E%3DCC%2BziN4df1Cdtb7Es-qc2KCFVnXrpF6w%40mail.gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQE-rMmuZ-UyzLyYkaS4kcT7UMqLRec-kzhKODA%2BzX8bAw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 12:30             ` Julia Lawall
@ 2015-10-22 12:33               ` Shivani Bhardwaj
  2015-10-22 12:37                 ` Julia Lawall
  0 siblings, 1 reply; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 12:33 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Thu, Oct 22, 2015 at 6:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>
>> On Thu, Oct 22, 2015 at 5:13 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >
>> >> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >
>> >> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >> >
>> >> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
>> >> >> >> as they are not required and replace their calls by suitable functions.
>> >> >> >
>> >> >> > The existing functions serve two purposes:
>> >> >> >
>> >> >> > 1.  They make clear that an ethernet address is being copied
>> >> >> > 2.  They hide the offsets
>> >> >> >
>> >> >> > To preserve the first property, the kernel has a function for copying
>> >> >> > ethernet addresses.
>> >> >> >
>> >> >> > To preserve the second property, maybe it would be possible to #define
>> >> >> > constants that describe the offsets, if there is any way to figure out
>> >> >> > what they semantically mean.
>> >> >> >
>> >> >> > julia
>> >> >> >
>> >> >>
>> >> >> Right. I am thinking of writing one wrapper function instead of three
>> >> >> where I'll pass on an offset each time which will be #defined.
>> >> >> For example,
>> >> >>
>> >> >> #define ofs 4
>> >> >> #define c 6
>> >> >
>> >> > #define things should always be in all (or at least mostly) capital
>> >> > letters.
>> >> >
>> >> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
>> >> >>         ether_addr_copy(addr, pum8sa + offset);
>> >> >> }
>> >> >
>> >> > I don't think this is the best idea.  ether_addr_copy is something that is
>> >> > easily understandable to someone reading the code.  get_address, on the
>> >> > other hand, could be anything.  If you can find meaningful names for the
>> >> > offsets, then it would be better to say eg:
>> >> >
>> >> > ether_addr_copy(addr, pum8sa + NAME1)
>> >> >
>> >> > julia
>> >> >
>> >>
>> >> #define NAME1 4
>> >> #define NAME2 10
>> >> #define NAME3 16
>> >
>> > Sorry, NAME was just supposed to be a metavariable, that you could fill in
>> > with something appropriate.  From the comments in the code it seems like
>> > it should be ADDRESS1, ADDRESS2, etc.
>> >
>> > julia
>> >
>> >
>>
>> Yes, I understand that. I was just presenting a prototype. :)
>> I was thinking of ADDR1, ADDR2.. But, ADDRESS1, ADDRESS2,.. would be fine too.
>
> Either way should be OK.
>
> julia
>
Thanks. I'm sending v2.
I'm getting implicit function declaration error for ether_addr_copy
function. Can it be ignored?

>>
>> Thank you, Julia! :)
>>
>> >>
>> >> func()
>> >> {
>> >>          if(...)
>> >>             ether_addr_copy(addr, pum8sa + NAME1);               //for address1
>> >>          else if(...)
>> >>              ether_addr_copy(addr, pum8sa + NAME2));          //for address2
>> >>          else
>> >>              ether_addr_copy(addr, pum8sa + NAME3);              //for address3
>> >> }
>> >>
>> >> Like this?
>> >>
>> >> >> func()
>> >> >> {
>> >> >>          if(...)
>> >> >>              get_address(data, bssid, ofs);               //for address1
>> >> >>          else if(...)
>> >> >>              get_address(data, bssid, ofs + c);          //for address2
>> >> >>          else
>> >> >>              get_address(data, bssid, ofs + 2*c);              //for address3
>> >> >> }
>> >> >>
>> >> >> This will make the work of function clear. Also, offset is not going
>> >> >> to be clear.
>> >> >> Is this going to be a good idea?
>> >> >>
>> >> >> Thank you
>> >> >>
>> >> >> >> Semantic patch used:
>> >> >> >> @@
>> >> >> >> identifier f,g;
>> >> >> >> @@
>> >> >> >>
>> >> >> >> *f(...) {
>> >> >> >>  g(...); }
>> >> >> >>
>> >> >> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
>> >> >> >> ---
>> >> >> >>  drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++-------------------
>> >> >> >>  1 file changed, 13 insertions(+), 25 deletions(-)
>> >> >> >>
>> >> >> >> diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> >> index 427adfd..38bc2c4 100644
>> >> >> >> --- a/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> >> +++ b/drivers/staging/wilc1000/coreconfigurator.c
>> >> >> >> @@ -171,38 +171,26 @@ static inline u8 get_from_ds(u8 *header)
>> >> >> >>       return ((header[1] & 0x02) >> 1);
>> >> >> >>  }
>> >> >> >>
>> >> >> >> -/* This function extracts the MAC Address in 'address1' field of the MAC     */
>> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> >> -static inline void get_address1(u8 *pu8msa, u8 *addr)
>> >> >> >> -{
>> >> >> >> -     memcpy(addr, pu8msa + 4, 6);
>> >> >> >> -}
>> >> >> >> -
>> >> >> >> -/* This function extracts the MAC Address in 'address2' field of the MAC     */
>> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> >> -static inline void get_address2(u8 *pu8msa, u8 *addr)
>> >> >> >> -{
>> >> >> >> -     memcpy(addr, pu8msa + 10, 6);
>> >> >> >> -}
>> >> >> >> -
>> >> >> >> -/* This function extracts the MAC Address in 'address3' field of the MAC     */
>> >> >> >> -/* header and updates the MAC Address in the allocated 'addr' variable.      */
>> >> >> >> -static inline void get_address3(u8 *pu8msa, u8 *addr)
>> >> >> >> -{
>> >> >> >> -     memcpy(addr, pu8msa + 16, 6);
>> >> >> >> -}
>> >> >> >> -
>> >> >> >>  /* This function extracts the BSSID from the incoming WLAN packet based on   */
>> >> >> >> -/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
>> >> >> >> +/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
>> >> >> >>  /* variable.                                                                 */
>> >> >> >>  static inline void get_BSSID(u8 *data, u8 *bssid)
>> >> >> >>  {
>> >> >> >>       if (get_from_ds(data) == 1)
>> >> >> >> -             get_address2(data, bssid);
>> >> >> >> +             /* Extract the MAC Address in 'address2' field of the MAC */
>> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> >> +              * variable. */
>> >> >> >> +             memcpy(data, bssid + 10, 6);
>> >> >> >>       else if (get_to_ds(data) == 1)
>> >> >> >> -             get_address1(data, bssid);
>> >> >> >> +             /* Extract the MAC Address in 'address1' field of the MAC */
>> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> >> +              * variable. */
>> >> >> >> +             memcpy(data, bssid + 4, 6);
>> >> >> >>       else
>> >> >> >> -             get_address3(data, bssid);
>> >> >> >> +             /* Extract the MAC Address in 'address3' field of the MAC */
>> >> >> >> +             /* header and update the MAC Address in the allocated 'data'
>> >> >> >> +              * variable. */
>> >> >> >> +             memcpy(data, bssid + 16, 6);
>> >> >> >>  }
>> >> >> >>
>> >> >> >>  /* This function extracts the SSID from a beacon/probe response frame        */
>> >> >> >> --
>> >> >> >> 2.1.0
>> >> >> >>
>> >> >> >> --
>> >> >> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> >> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> >> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151022101158.GA17223%40ubuntu.
>> >> >> >> For more options, visit https://groups.google.com/d/optout.
>> >> >> >>
>> >> >>
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQHP7dpLZdz6E%3DCC%2BziN4df1Cdtb7Es-qc2KCFVnXrpF6w%40mail.gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQE-rMmuZ-UyzLyYkaS4kcT7UMqLRec-kzhKODA%2BzX8bAw%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 12:33               ` Shivani Bhardwaj
@ 2015-10-22 12:37                 ` Julia Lawall
  2015-10-22 12:38                   ` Shivani Bhardwaj
  0 siblings, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2015-10-22 12:37 UTC (permalink / raw)
  To: Shivani Bhardwaj; +Cc: outreachy-kernel



On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:

> On Thu, Oct 22, 2015 at 6:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> On Thu, Oct 22, 2015 at 5:13 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> >
> >> >
> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >
> >> >> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >> >
> >> >> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
> >> >> >> >
> >> >> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
> >> >> >> >> as they are not required and replace their calls by suitable functions.
> >> >> >> >
> >> >> >> > The existing functions serve two purposes:
> >> >> >> >
> >> >> >> > 1.  They make clear that an ethernet address is being copied
> >> >> >> > 2.  They hide the offsets
> >> >> >> >
> >> >> >> > To preserve the first property, the kernel has a function for copying
> >> >> >> > ethernet addresses.
> >> >> >> >
> >> >> >> > To preserve the second property, maybe it would be possible to #define
> >> >> >> > constants that describe the offsets, if there is any way to figure out
> >> >> >> > what they semantically mean.
> >> >> >> >
> >> >> >> > julia
> >> >> >> >
> >> >> >>
> >> >> >> Right. I am thinking of writing one wrapper function instead of three
> >> >> >> where I'll pass on an offset each time which will be #defined.
> >> >> >> For example,
> >> >> >>
> >> >> >> #define ofs 4
> >> >> >> #define c 6
> >> >> >
> >> >> > #define things should always be in all (or at least mostly) capital
> >> >> > letters.
> >> >> >
> >> >> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
> >> >> >>         ether_addr_copy(addr, pum8sa + offset);
> >> >> >> }
> >> >> >
> >> >> > I don't think this is the best idea.  ether_addr_copy is something that is
> >> >> > easily understandable to someone reading the code.  get_address, on the
> >> >> > other hand, could be anything.  If you can find meaningful names for the
> >> >> > offsets, then it would be better to say eg:
> >> >> >
> >> >> > ether_addr_copy(addr, pum8sa + NAME1)
> >> >> >
> >> >> > julia
> >> >> >
> >> >>
> >> >> #define NAME1 4
> >> >> #define NAME2 10
> >> >> #define NAME3 16
> >> >
> >> > Sorry, NAME was just supposed to be a metavariable, that you could fill in
> >> > with something appropriate.  From the comments in the code it seems like
> >> > it should be ADDRESS1, ADDRESS2, etc.
> >> >
> >> > julia
> >> >
> >> >
> >>
> >> Yes, I understand that. I was just presenting a prototype. :)
> >> I was thinking of ADDR1, ADDR2.. But, ADDRESS1, ADDRESS2,.. would be fine too.
> >
> > Either way should be OK.
> >
> > julia
> >
> Thanks. I'm sending v2.
> I'm getting implicit function declaration error for ether_addr_copy
> function. Can it be ignored?

No.  You need to find the appropriate header file to include.

julia


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Outreachy kernel] [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
  2015-10-22 12:37                 ` Julia Lawall
@ 2015-10-22 12:38                   ` Shivani Bhardwaj
  0 siblings, 0 replies; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 12:38 UTC (permalink / raw)
  To: Julia Lawall; +Cc: outreachy-kernel

On Thu, Oct 22, 2015 at 6:07 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>
>> On Thu, Oct 22, 2015 at 6:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >
>> >> On Thu, Oct 22, 2015 at 5:13 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> >
>> >> >
>> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >
>> >> >> On Thu, Oct 22, 2015 at 4:43 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >> >
>> >> >> >> On Thu, Oct 22, 2015 at 4:04 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >> >> >> > On Thu, 22 Oct 2015, Shivani Bhardwaj wrote:
>> >> >> >> >
>> >> >> >> >> Remove the wrapper functions get_address1, get_address2 and get_address3
>> >> >> >> >> as they are not required and replace their calls by suitable functions.
>> >> >> >> >
>> >> >> >> > The existing functions serve two purposes:
>> >> >> >> >
>> >> >> >> > 1.  They make clear that an ethernet address is being copied
>> >> >> >> > 2.  They hide the offsets
>> >> >> >> >
>> >> >> >> > To preserve the first property, the kernel has a function for copying
>> >> >> >> > ethernet addresses.
>> >> >> >> >
>> >> >> >> > To preserve the second property, maybe it would be possible to #define
>> >> >> >> > constants that describe the offsets, if there is any way to figure out
>> >> >> >> > what they semantically mean.
>> >> >> >> >
>> >> >> >> > julia
>> >> >> >> >
>> >> >> >>
>> >> >> >> Right. I am thinking of writing one wrapper function instead of three
>> >> >> >> where I'll pass on an offset each time which will be #defined.
>> >> >> >> For example,
>> >> >> >>
>> >> >> >> #define ofs 4
>> >> >> >> #define c 6
>> >> >> >
>> >> >> > #define things should always be in all (or at least mostly) capital
>> >> >> > letters.
>> >> >> >
>> >> >> >> static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){
>> >> >> >>         ether_addr_copy(addr, pum8sa + offset);
>> >> >> >> }
>> >> >> >
>> >> >> > I don't think this is the best idea.  ether_addr_copy is something that is
>> >> >> > easily understandable to someone reading the code.  get_address, on the
>> >> >> > other hand, could be anything.  If you can find meaningful names for the
>> >> >> > offsets, then it would be better to say eg:
>> >> >> >
>> >> >> > ether_addr_copy(addr, pum8sa + NAME1)
>> >> >> >
>> >> >> > julia
>> >> >> >
>> >> >>
>> >> >> #define NAME1 4
>> >> >> #define NAME2 10
>> >> >> #define NAME3 16
>> >> >
>> >> > Sorry, NAME was just supposed to be a metavariable, that you could fill in
>> >> > with something appropriate.  From the comments in the code it seems like
>> >> > it should be ADDRESS1, ADDRESS2, etc.
>> >> >
>> >> > julia
>> >> >
>> >> >
>> >>
>> >> Yes, I understand that. I was just presenting a prototype. :)
>> >> I was thinking of ADDR1, ADDR2.. But, ADDRESS1, ADDRESS2,.. would be fine too.
>> >
>> > Either way should be OK.
>> >
>> > julia
>> >
>> Thanks. I'm sending v2.
>> I'm getting implicit function declaration error for ether_addr_copy
>> function. Can it be ignored?
>
> No.  You need to find the appropriate header file to include.
>
> julia

All right.
Thank you


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
@ 2015-10-22 12:53 Shivani Bhardwaj
  0 siblings, 0 replies; 12+ messages in thread
From: Shivani Bhardwaj @ 2015-10-22 12:53 UTC (permalink / raw)
  To: outreachy-kernel

Remove the wrapper functions get_address1, get_address2 and get_address3
as they are not required and replace their calls by suitable functions.
Semantic patch used:

@@
identifier f,g;
@@

*f(...) {
 g(...); }

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 drivers/staging/wilc1000/coreconfigurator.c | 48 ++++++++++++++---------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c b/drivers/staging/wilc1000/coreconfigurator.c
index 427adfd..6dfe26d 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -13,8 +13,12 @@
 #include "wilc_wlan.h"
 #include <linux/errno.h>
 #include <linux/slab.h>
+#include <linux/etherdevice.h>
 #define TAG_PARAM_OFFSET	(MAC_HDR_LEN + TIME_STAMP_LEN + \
 							BEACON_INTERVAL_LEN + CAP_INFO_LEN)
+#define ADDR1 4
+#define ADDR2 10
+#define ADDR3 16
 
 /* Basic Frame Type Codes (2-bit) */
 enum basic_frame_type {
@@ -171,38 +175,32 @@ static inline u8 get_from_ds(u8 *header)
 	return ((header[1] & 0x02) >> 1);
 }
 
-/* This function extracts the MAC Address in 'address1' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address1(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 4, 6);
-}
-
-/* This function extracts the MAC Address in 'address2' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address2(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 10, 6);
-}
-
-/* This function extracts the MAC Address in 'address3' field of the MAC     */
-/* header and updates the MAC Address in the allocated 'addr' variable.      */
-static inline void get_address3(u8 *pu8msa, u8 *addr)
-{
-	memcpy(addr, pu8msa + 16, 6);
-}
-
 /* This function extracts the BSSID from the incoming WLAN packet based on   */
-/* the 'from ds' bit, and updates the MAC Address in the allocated 'addr'    */
+/* the 'from ds' bit, and updates the MAC Address in the allocated 'data'    */
 /* variable.                                                                 */
 static inline void get_BSSID(u8 *data, u8 *bssid)
 {
 	if (get_from_ds(data) == 1)
-		get_address2(data, bssid);
+		/*
+		 * Extract the MAC Address in 'address2' field of the MAC
+		 * header and update the MAC Address in the allocated 'data'
+		 *  variable.
+		 */
+		ether_addr_copy(data, bssid + ADDR2);
 	else if (get_to_ds(data) == 1)
-		get_address1(data, bssid);
+		/*
+		 * Extract the MAC Address in 'address1' field of the MAC
+		 * header and update the MAC Address in the allocated 'data'
+		 * variable.
+		 */
+		ether_addr_copy(data, bssid + ADDR1);
 	else
-		get_address3(data, bssid);
+		/*
+		 * Extract the MAC Address in 'address3' field of the MAC
+		 * header and update the MAC Address in the allocated 'data'
+		 * variable.
+		 */
+		ether_addr_copy(data, bssid + ADDR3);
 }
 
 /* This function extracts the SSID from a beacon/probe response frame        */
-- 
2.1.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-10-22 12:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-22 10:11 [PATCH] Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions Shivani Bhardwaj
2015-10-22 10:34 ` [Outreachy kernel] " Julia Lawall
2015-10-22 11:01   ` Shivani Bhardwaj
2015-10-22 11:13     ` Julia Lawall
2015-10-22 11:28       ` Shivani Bhardwaj
2015-10-22 11:43         ` Julia Lawall
2015-10-22 12:26           ` Shivani Bhardwaj
2015-10-22 12:30             ` Julia Lawall
2015-10-22 12:33               ` Shivani Bhardwaj
2015-10-22 12:37                 ` Julia Lawall
2015-10-22 12:38                   ` Shivani Bhardwaj
  -- strict thread matches above, loose matches on Subject: below --
2015-10-22 12:53 Shivani Bhardwaj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.