staging: rtl8188eu: core: constify local structure

PATCH In review Elizabeth Ferdman 2 participants 5 messages Oct 17, 2016 – Oct 19, 2016 Staging

lore

Share & tools

lore

Elizabeth Ferdman

author

2016-10-17 06:46 UTC #0 lore raw

PATCH 1/1 staging: rtl8188eu: core: constify local structure

Constify the static struct RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.

The only other instance was one of its properties being assigned to a variable:
Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;

Therefore this is a good candidate for constification.

Running size did not show any difference:
Before/After:

text    data     bss     dec     hex filename
44122     408    2974   47504    b990

Lastly, break up the long line to fix the checkpatch warning and conform to kernel coding style.

Signed-off-by Elizabeth Ferdman <[email protected]>

---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +3 -1

index fb13df5..c94700c 100644
@@ -133,7 +133,9 @@ static struct rt_channel_plan_map	RTW_ChannelPlanMap[RT_CHANNEL_DOMAIN_MAX] = {
 	{0x03},	/* 0x41, RT_CHANNEL_DOMAIN_GLOBAL_DOAMIN_2G */
 };
 
-static struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {0x03}; /* use the combination for max channel numbers */
+static const struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {
+	0x03
+}; /* use the combination for max channel numbers */
 
 /*
  * Search the @param channel_num in given @param channel_set

Signature · 1 line

2.1.4

Julia Lawall

2016-10-17 09:26 UTC #1 lore raw

Quoted 6 lines · Elizabeth Ferdman

Constify the static struct RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.
The only other instance was one of its properties being assigned to a variable:
Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;

Quoted 6 lines

Therefore this is a good candidate for constification.
Running size did not show any difference:
Before/After:
text    data     bss     dec     hex filename
44122     408    2974   47504    b990

This is not a good sign. It suggests that you have not actually compiled the lines of code that you changed.

It is a little bit strange that a structure is declared to only contain the number 3.

I wonder if something else is going on?

julia

Quoted 34 lines

Lastly, break up the long line to fix the checkpatch warning and conform to kernel coding style.
Signed-off-by Elizabeth Ferdman <[email protected]>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +3 -2
index fb13df5..c94700c 100644
@@ -133,7 +133,9 @@ static struct rt_channel_plan_map	RTW_ChannelPlanMap[RT_CHANNEL_DOMAIN_MAX] = {
 	{0x03},	/* 0x41, RT_CHANNEL_DOMAIN_GLOBAL_DOAMIN_2G */
 };
 
-static struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {0x03}; /* use the combination for max channel numbers */
+static const struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {
+	0x03
+}; /* use the combination for max channel numbers */
 
 /*
  * Search the @param channel_num in given @param channel_set
--
2.1.4
--
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/20161017064620.GA21653%40localhost.
For more options, visit https://groups.google.com/d/optout.

Elizabeth Ferdman

author

2016-10-17 22:23 UTC #2 lore raw

Quoted 12 lines · Julia Lawall

Quoted 8 lines · Elizabeth Ferdman
Constify the static struct RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.
The only other instance was one of its properties being assigned to a variable:
Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
Therefore this is a good candidate for constification.

Quoted 8 lines

Quoted 4 lines
Running size did not show any difference:
Before/After:
text    data     bss     dec     hex filename
44122     408    2974   47504    b990
This is not a good sign. It suggests that you have not actually compiled the lines of code that you changed.

That is indeed strange, I don't understand why that happened. I tried it again, and got this:

Before:
text data bss dec hex filename
44122 408 2974 47504 b990

After:

text    data     bss     dec     hex filename
 68238     408    5398   74044   1213c

Quoted 3 lines

It is a little bit strange that a structure is declared to only contain the number 3.

Yeah, this is how it's defined:

struct rt_channel_plan_map {
       unsigned char   Index2G;
};

Is it unconventional to have a struct with
only one member? I guess it could just be an unsigned char. Perhaps it is to differentiate it from the other variable that is also called Index2G so maybe he's using it as a namespace.

if (RT_CHANNEL_DOMAIN_REALTEK_DEFINE == ChannelPlIndex2G)

	Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
else
	Index2G = RTW_ChannelPlanMap[ChannelPlan].Index2G;

Quoted 38 lines

I wonder if something else is going on?
julia
Quoted 34 lines
Lastly, break up the long line to fix the checkpatch warning and conform to kernel coding style.
Signed-off-by Elizabeth Ferdman <[email protected]>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +3 -2
index fb13df5..c94700c 100644
@@ -133,7 +133,9 @@ static struct rt_channel_plan_map	RTW_ChannelPlanMap[RT_CHANNEL_DOMAIN_MAX] = {
 	{0x03},	/* 0x41, RT_CHANNEL_DOMAIN_GLOBAL_DOAMIN_2G */
 };
 
-static struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {0x03}; /* use the combination for max channel numbers */
+static const struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {
+	0x03
+}; /* use the combination for max channel numbers */
 
 /*
  * Search the @param channel_num in given @param channel_set
--
2.1.4
--
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/20161017064620.GA21653%40localhost.
For more options, visit https://groups.google.com/d/optout.

Julia Lawall

2016-10-18 05:26 UTC #3 lore raw

Quoted 25 lines · Elizabeth Ferdman

Quoted 12 lines · Julia Lawall
Quoted 8 lines · Elizabeth Ferdman
Constify the static struct RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.
The only other instance was one of its properties being assigned to a variable:
Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
Therefore this is a good candidate for constification.
Quoted 8 lines
Quoted 4 lines
Running size did not show any difference:
Before/After:
text    data     bss     dec     hex filename
44122     408    2974   47504    b990
This is not a good sign. It suggests that you have not actually compiled the lines of code that you changed.
That is indeed strange, I don't understand why that happened. I tried it again, and got this:

Quoted 7 lines

Before:
text data bss dec hex filename
44122 408 2974 47504 b990
After:
text    data     bss     dec     hex filename
 68238     408    5398   74044   1213c

This is even odder. Now the data size has not changed (it should go down) and the other numbers have changed massively.

julia

Quoted 68 lines

Quoted 4 lines
It is a little bit strange that a structure is declared to only contain the number 3.
Yeah, this is how it's defined:
struct rt_channel_plan_map {
       unsigned char   Index2G;
};
Is it unconventional to have a struct with
only one member? I guess it could just be an unsigned char. Perhaps it is to differentiate it from the other variable that is also called Index2G so maybe he's using it as a namespace.
if (RT_CHANNEL_DOMAIN_REALTEK_DEFINE == ChannelPlIndex2G)
	Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
else
	Index2G = RTW_ChannelPlanMap[ChannelPlan].Index2G;
Quoted 31 lines
I wonder if something else is going on?
julia
Quoted 27 lines
Lastly, break up the long line to fix the checkpatch warning and conform to kernel coding style.
Signed-off-by Elizabeth Ferdman <[email protected]>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +3 -2
index fb13df5..c94700c 100644
@@ -133,7 +133,9 @@ static struct rt_channel_plan_map	RTW_ChannelPlanMap[RT_CHANNEL_DOMAIN_MAX] = {
 	{0x03},	/* 0x41, RT_CHANNEL_DOMAIN_GLOBAL_DOAMIN_2G */
 };
 
-static struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {0x03}; /* use the combination for max channel numbers */
+static const struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {
+	0x03
+}; /* use the combination for max channel numbers */
 
 /*
  * Search the @param channel_num in given @param channel_set
--
2.1.4
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/20161017064620.GA21653%40localhost.
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/20161017222305.GB2705%40localhost.
For more options, visit https://groups.google.com/d/optout.

Elizabeth Ferdman

author

2016-10-19 22:55 UTC #4 lore raw

Quoted 33 lines · Julia Lawall

Quoted 29 lines · Elizabeth Ferdman
Quoted 12 lines · Julia Lawall
Quoted 8 lines · Elizabeth Ferdman
Constify the static struct RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.
The only other instance was one of its properties being assigned to a variable:
Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
Therefore this is a good candidate for constification.
Quoted 8 lines
Quoted 4 lines
Running size did not show any difference:
Before/After:
text    data     bss     dec     hex filename
44122     408    2974   47504    b990
This is not a good sign. It suggests that you have not actually compiled the lines of code that you changed.
That is indeed strange, I don't understand why that happened. I tried it again, and got this:
Before:
text data bss dec hex filename
44122 408 2974 47504 b990

Quoted 7 lines

Quoted 3 lines
After:
text    data     bss     dec     hex filename
 68238     408    5398   74044   1213c
This is even odder. Now the data size has not changed (it should go down) and the other numbers have changed massively.

I tried it again. The text shouldnt have changed so massively.It should have just gone from 68238 to 68286. The data stays constant at 408. I ran "make allyesconfig drivers/staging/rtl8188eu/" and saw a CC line which confirmed that the specific .o file was compiled. I did ls -lt on the folder to confirm that the .o file was current. No clue why the data isn't going down :/. Could it be because the struct only holds one number?

Quoted 70 lines

julia
Quoted 68 lines
Quoted 4 lines
It is a little bit strange that a structure is declared to only contain the number 3.
Yeah, this is how it's defined:
struct rt_channel_plan_map {
       unsigned char   Index2G;
};
Is it unconventional to have a struct with
only one member? I guess it could just be an unsigned char. Perhaps it is to differentiate it from the other variable that is also called Index2G so maybe he's using it as a namespace.
if (RT_CHANNEL_DOMAIN_REALTEK_DEFINE == ChannelPlIndex2G)
	Index2G = RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE.Index2G;
else
	Index2G = RTW_ChannelPlanMap[ChannelPlan].Index2G;
Quoted 31 lines
I wonder if something else is going on?
julia
Quoted 27 lines
Lastly, break up the long line to fix the checkpatch warning and conform to kernel coding style.
Signed-off-by Elizabeth Ferdman <[email protected]>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +3 -2
index fb13df5..c94700c 100644
@@ -133,7 +133,9 @@ static struct rt_channel_plan_map	RTW_ChannelPlanMap[RT_CHANNEL_DOMAIN_MAX] = {
 	{0x03},	/* 0x41, RT_CHANNEL_DOMAIN_GLOBAL_DOAMIN_2G */
 };
 
-static struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {0x03}; /* use the combination for max channel numbers */
+static const struct rt_channel_plan_map RTW_CHANNEL_PLAN_MAP_REALTEK_DEFINE = {
+	0x03
+}; /* use the combination for max channel numbers */
 
 /*
  * Search the @param channel_num in given @param channel_set
--
2.1.4
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/20161017064620.GA21653%40localhost.
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/20161017222305.GB2705%40localhost.
For more options, visit https://groups.google.com/d/optout.