Re: [Outreachy kernel] [Outreachy Kernel] [PATCH] staging: greybus: Constify local struct gb_loopback_stats

[Rehas Sachdeva <aquannie@gmail.com>]

On Mon, Oct 03, 2016 at 11:16:03AM +0100, Bryan O'Donoghue wrote:
> On Sun, 2016-10-02 at 21:36 +0200, Julia Lawall wrote:
> > 
> > On Mon, 3 Oct 2016, Rehas Sachdeva wrote:
> > 
> > > The local declaration "struct gb_loopback_stats reset" is not
> > modified
> > > throughout its scope and hence can be made const. Done using
> > Coccinelle.
> > >
> > > Signed-off-by: Rehas Sachdeva <aquannie@gmail.com>
> > > ---
> > >� drivers/staging/greybus/loopback.c | 2 +-
> > >� 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/greybus/loopback.c
> > b/drivers/staging/greybus/loopback.c
> > > index 7882306..fb5a068 100644
> > > --- a/drivers/staging/greybus/loopback.c
> > > +++ b/drivers/staging/greybus/loopback.c
> > > @@ -837,7 +837,7 @@ static int gb_loopback_request_handler(struct
> > gb_operation *operation)
> > >
> > >� static void gb_loopback_reset_stats(struct gb_loopback *gb)
> > >� {
> > > -�����struct gb_loopback_stats reset = {
> > > +�����const struct gb_loopback_stats reset = {
> > 
> > Should it also be static?� I recall this comment being made in
> > another
> > case, but I don't know what the tradeoffs are.
> 

Yes it can be static too, since we are not updating it, so we don't care
if its value is retained in subsequent function calls. I agree with Bryan that
we might as well make all locally scoped const variables static too.

Making it static decreases the size of text segment, without affecting any
other segment. I don't know if its particularly useful.

Rehas

> const yes agree - good catch.
> 
> static ... can't see a reason why TBH. Making this variable static we
> might as well declare all locally scoped variables static.
> 
> ---
> bod