* [PATCH] staging: rtl8188eu: core: constify local structure @ 2016-10-17 6:46 Elizabeth Ferdman 2016-10-17 9:26 ` [Outreachy kernel] " Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Elizabeth Ferdman @ 2016-10-17 6:46 UTC (permalink / raw) To: outreachy-kernel; +Cc: gregkh 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 <gnudevliz@gmail.com> --- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index fb13df5..c94700c 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8188eu: core: constify local structure 2016-10-17 6:46 [PATCH] staging: rtl8188eu: core: constify local structure Elizabeth Ferdman @ 2016-10-17 9:26 ` Julia Lawall 2016-10-17 22:23 ` Elizabeth Ferdman 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2016-10-17 9:26 UTC (permalink / raw) To: Elizabeth Ferdman; +Cc: outreachy-kernel, gregkh On Sun, 16 Oct 2016, Elizabeth Ferdman wrote: > 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 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 > > Lastly, break up the long line to fix the checkpatch warning and conform > to kernel coding style. > > Signed-off-by: Elizabeth Ferdman <gnudevliz@gmail.com> > --- > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > index fb13df5..c94700c 100644 > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > @@ -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 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/20161017064620.GA21653%40localhost. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8188eu: core: constify local structure 2016-10-17 9:26 ` [Outreachy kernel] " Julia Lawall @ 2016-10-17 22:23 ` Elizabeth Ferdman 2016-10-18 5:26 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Elizabeth Ferdman @ 2016-10-17 22:23 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel On Mon, Oct 17, 2016 at 11:26:11AM +0200, Julia Lawall wrote: > > > On Sun, 16 Oct 2016, Elizabeth Ferdman wrote: > > > 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 > > 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 > 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; > I wonder if something else is going on? > > julia > > > > > Lastly, break up the long line to fix the checkpatch warning and conform > > to kernel coding style. > > > > Signed-off-by: Elizabeth Ferdman <gnudevliz@gmail.com> > > --- > > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > index fb13df5..c94700c 100644 > > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > @@ -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 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/20161017064620.GA21653%40localhost. > > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8188eu: core: constify local structure 2016-10-17 22:23 ` Elizabeth Ferdman @ 2016-10-18 5:26 ` Julia Lawall 2016-10-19 22:55 ` Elizabeth Ferdman 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2016-10-18 5:26 UTC (permalink / raw) To: Elizabeth Ferdman; +Cc: outreachy-kernel On Mon, 17 Oct 2016, Elizabeth Ferdman wrote: > On Mon, Oct 17, 2016 at 11:26:11AM +0200, Julia Lawall wrote: > > > > > > On Sun, 16 Oct 2016, Elizabeth Ferdman wrote: > > > > > 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 > > > > 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 This is even odder. Now the data size has not changed (it should go down) and the other numbers have changed massively. julia > > > 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; > > > > I wonder if something else is going on? > > > > julia > > > > > > > > Lastly, break up the long line to fix the checkpatch warning and conform > > > to kernel coding style. > > > > > > Signed-off-by: Elizabeth Ferdman <gnudevliz@gmail.com> > > > --- > > > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > index fb13df5..c94700c 100644 > > > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > @@ -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 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/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 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/20161017222305.GB2705%40localhost. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8188eu: core: constify local structure 2016-10-18 5:26 ` Julia Lawall @ 2016-10-19 22:55 ` Elizabeth Ferdman 0 siblings, 0 replies; 5+ messages in thread From: Elizabeth Ferdman @ 2016-10-19 22:55 UTC (permalink / raw) To: Julia Lawall; +Cc: outreachy-kernel On Tue, Oct 18, 2016 at 07:26:47AM +0200, Julia Lawall wrote: > > > On Mon, 17 Oct 2016, Elizabeth Ferdman wrote: > > > On Mon, Oct 17, 2016 at 11:26:11AM +0200, Julia Lawall wrote: > > > > > > > > > On Sun, 16 Oct 2016, Elizabeth Ferdman wrote: > > > > > > > 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 > > > > > > 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 > > 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? > julia > > > > > > 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; > > > > > > > I wonder if something else is going on? > > > > > > julia > > > > > > > > > > > Lastly, break up the long line to fix the checkpatch warning and conform > > > > to kernel coding style. > > > > > > > > Signed-off-by: Elizabeth Ferdman <gnudevliz@gmail.com> > > > > --- > > > > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > > index fb13df5..c94700c 100644 > > > > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > > > @@ -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 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/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 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/20161017222305.GB2705%40localhost. > > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-19 22:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-17 6:46 [PATCH] staging: rtl8188eu: core: constify local structure Elizabeth Ferdman 2016-10-17 9:26 ` [Outreachy kernel] " Julia Lawall 2016-10-17 22:23 ` Elizabeth Ferdman 2016-10-18 5:26 ` Julia Lawall 2016-10-19 22:55 ` Elizabeth Ferdman
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.