Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
PATCH 1/1 Staging: wilc1000: coreconfigurator: Drop unneeded wrapper functions
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(...); }
--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)
drivers/staging/wilc1000/coreconfigurator.c +13 -25
@@ -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 */
Signature · 1 line
2.1.0
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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
Quoted 9 lines
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
Quoted 79 lines
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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
Quoted 96 lines
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
Quoted 79 lines
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 37 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
static inline void get_address(u8 *pum8sa, u8 *addr, u8 *offset){ ether_addr_copy(addr, pum8sa + offset); }
Quoted 9 lines
I don't think this is the best idea.
ether_addr_copyis 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?
Quoted 96 lines
Quoted 96 lines
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
Quoted 79 lines
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:
Quoted 8 lines
Quoted 4 lines
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
Quoted 117 lines
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?
Quoted 88 lines
Quoted 88 lines
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
Quoted 72 lines
Quoted 72 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
--
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 [email protected]. To post to this group, send email to [email protected].
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 [email protected].
To post to this group, send email to [email protected].
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.
Quoted 56 lines · Julia Lawall
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:Quoted 8 lines
Quoted 4 lines
ether_addr_copy(addr, pum8sa + NAME1)julia
#define NAME1 4
#define NAME2 10
#define NAME3 16
Quoted 7 lines
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! :)
Quoted 117 lines
Quoted 117 lines
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?
Quoted 88 lines
Quoted 88 lines
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
Quoted 72 lines
Quoted 72 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
--
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 [email protected]. To post to this group, send email to [email protected].
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 [email protected].
To post to this group, send email to [email protected].
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.
Quoted 61 lines · Shivani Bhardwaj
Quoted 60 lines · Julia Lawall
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:Quoted 8 lines
Quoted 4 lines
ether_addr_copy(addr, pum8sa + NAME1)julia
#define NAME1 4
#define NAME2 10
#define NAME3 16Sorry, 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.
Quoted 6 lines
Quoted 3 lines
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
Quoted 128 lines
Thank you, Julia! :)
Quoted 110 lines
Quoted 110 lines
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?
Quoted 96 lines
Quoted 96 lines
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
Quoted 79 lines
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.Quoted 7 lines
Quoted 7 lines
--
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 [email protected].
To post to this group, send email to [email protected].
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 [email protected].
To post to this group, send email to [email protected].
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.
Quoted 69 lines · Julia Lawall
Quoted 65 lines · Shivani Bhardwaj
Quoted 63 lines · Julia Lawall
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:Quoted 8 lines
Quoted 4 lines
ether_addr_copy(addr, pum8sa + NAME1)julia
#define NAME1 4
#define NAME2 10
#define NAME3 16Sorry, 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
Quoted 7 lines
Quoted 2 lines
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?
Quoted 128 lines
Quoted 128 lines
Thank you, Julia! :)
Quoted 110 lines
Quoted 110 lines
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?
Quoted 96 lines
Quoted 96 lines
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
Quoted 79 lines
Quoted 79 lines
Semantic patch used:
@@
identifier f,g;
@@*f(...) {
g(...); }Signed-off-by Shivani Bhardwaj <[email protected]>--- drivers/staging/wilc1000/coreconfigurator.c | 38 ++++++++++------------------- 1 file changed, 13 insertions(+), 25 deletions(-)drivers/staging/wilc1000/coreconfigurator.c +13
-26@@ -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 [email protected]. To post to this group, send email to [email protected].
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.Quoted 7 lines
Quoted 7 lines
--
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 [email protected].
To post to this group, send email to [email protected].
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 [email protected].
To post to this group, send email to [email protected].
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.
Quoted 73 lines · Shivani Bhardwaj
Quoted 72 lines · Julia Lawall
Quoted 67 lines · Shivani Bhardwaj
Quoted 56 lines · Julia Lawall
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:Quoted 8 lines
Quoted 4 lines
ether_addr_copy(addr, pum8sa + NAME1)julia
#define NAME1 4
#define NAME2 10
#define NAME3 16Quoted 7 lines
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.
Quoted 7 lines
Quoted 4 lines
Either way should be OK.
julia
Thanks. I'm sending v2.
I'm getting implicit function declaration error forether_addr_copyfunction. Can it be ignored?
No. You need to find the appropriate header file to include.
julia
Quoted 81 lines · Julia Lawall
Quoted 77 lines · Shivani Bhardwaj
Quoted 76 lines · Julia Lawall
Quoted 61 lines · Shivani Bhardwaj
Quoted 60 lines · Julia Lawall
Quoted 43 lines · Shivani Bhardwaj
Quoted 42 lines · Julia Lawall
Quoted 21 lines · Shivani Bhardwaj
Quoted 10 lines · Julia Lawall
Quoted 2 lines · Shivani Bhardwaj
Remove the wrapper functions
get_address1,get_address2andget_address3as 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 offsetsQuoted 9 lines
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
Quoted 6 lines
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.
Quoted 3 lines
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_copyis 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:Quoted 8 lines
Quoted 4 lines
ether_addr_copy(addr, pum8sa + NAME1)julia
#define NAME1 4
#define NAME2 10
#define NAME3 16Sorry, 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.
Quoted 6 lines
Quoted 3 lines
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
Quoted 7 lines
Quoted 3 lines
Thanks. I'm sending v2.
I'm getting implicit function declaration error forether_addr_copyfunction. Can it be ignored?No. You need to find the appropriate header file to include.
julia
All right.
Thank you