* [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.