Re: [Outreachy kernel] [PATCH] staging: sm750fb: make pointers in array const

["Fabio M. De Francesco" <fmdefrancesco@gmail.com>]

On Monday, October 18, 2021 7:01:42 PM CEST Julia Lawall wrote:
> 
> On Mon, 18 Oct 2021, Fabio M. De Francesco wrote:
> 
> > On Monday, October 18, 2021 6:44:31 PM CEST Kushal Kothari wrote:
> > > Change the parameters of functions from const char *g_fbmode[] to
> > > const char * const g_fbmode[]. This additional const is needed to
> > > allow us to fix  checkpatch warning, as well as being good
> > > programming practice.
> > >
> > > For the checkpatch warnings, if we have a set of command line
> > > args that we want to check defined as:
> > > 	static const char * g_fbmode[] = {NULL, NULL};
> > >
> > > checkpatch will complain:
> > > 	WARNING: static const char * array should probably be static 
const
> > char * const
> > >
> > > Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> > > ---
> > >
> > > [...]
> > >
> > > -static const char *g_fbmode[] = {NULL, NULL};
> > > +static const char * const g_fbmode[] = {NULL, NULL};
> >
> > You have introduced a logical change (g_fbmode[] entries cannot be 
assigned
> > any more) and a build error (because there is code somewhere that assigns
> > values to those slots).
> 
> I wonder if this warning makes much sense when the array elements are
> NULL.  I don't know if checkpatch could easily detect that.
> 
> julia

Unfortunately I cannot change checkpatch.pl because I am not able to program 
with Perl.

However, it should be an easy fix in whatever programming language: 
checkpatch should warn if and only if the array elements are assigned with 
non 'NULL' values, because it's pretty clear that somewhere else there must 
be some lines that assign values to them.

Fabio
 
> 
> >
> > Please, build the code after you've changed it.
> >
> > Thanks,
> >
> > Fabio
> >
> > >  static const char *g_def_fbmode = "1024x768-32@60";
> > >  static char *g_settings;
> > >  static int g_dualview;
> > > --
> > > 2.25.1
> > >