Re: [Outreachy kernel] [PATCH] staging: rtl8188eu: core: constify local structure

[Elizabeth Ferdman <gnudevliz@gmail.com>]

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