linux-kernel - Re: [PATCH] staging: sm750fb: fix instances of camel case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6ec1fa494ee823549fb97a48121cb28e37f1cc4d.camel@aol.com>
Date: Thu, 17 Apr 2025 18:28:07 +0100
From: Ruben Wauters <rubenru09@....com>
To: Greg Korah-Hartman <gregkh@...uxfoundation.org>
Cc: Sudip Mukherjee <sudipm.mukherjee@...il.com>, Teddy Wang
	 <teddy.wang@...iconmotion.com>, Sudip Mukherjee
	 <sudip.mukherjee@...ethink.co.uk>, linux-fbdev@...r.kernel.org, 
	linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: sm750fb: fix instances of camel case

On Thu, 2025-04-17 at 18:58 +0200, Greg Korah-Hartman wrote:
> On Thu, Apr 17, 2025 at 04:27:47PM +0100, Ruben Wauters wrote:
> > As per the kernel style guidelines, and as reported by
> > checkpatch.pl,
> > replaced instances of camel case with snake_case where appropriate
> > and
> > aligned names in the header with those in the c file.
> > 
> > Signed-off-by: Ruben Wauters <rubenru09@....com>
> > ---
> >  drivers/staging/sm750fb/ddk750_sii164.c | 113 ++++++++++++--------
> > ----
> >  drivers/staging/sm750fb/ddk750_sii164.h |  26 +++---
> >  2 files changed, 69 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/staging/sm750fb/ddk750_sii164.c
> > b/drivers/staging/sm750fb/ddk750_sii164.c
> > index 89700fc5dd2e..20c2f386220c 100644
> > --- a/drivers/staging/sm750fb/ddk750_sii164.c
> > +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> > @@ -12,11 +12,11 @@
> >  #define USE_HW_I2C
> >  
> >  #ifdef USE_HW_I2C
> > -    #define i2cWriteReg sm750_hw_i2c_write_reg
> > -    #define i2cReadReg  sm750_hw_i2c_read_reg
> > +    #define I2C_WRITE_REG sm750_hw_i2c_write_reg
> > +    #define I2C_READ_REG  sm750_hw_i2c_read_reg
> 
> Close, but these are really a function name, not a macro, right?
> 
> And what sets this #define?  If it's always enabled, then unwrap this
> indirection instead of keeping it around

Will take a look into it, if it turns out that this is in fact actually
used/different, what would be the best way to name this? checkpatch.pl
doesn't like the camelCase that's currently there.

> >  #else
> > -    #define i2cWriteReg sm750_sw_i2c_write_reg
> > -    #define i2cReadReg  sm750_sw_i2c_read_reg
> > +    #define I2C_WRITE_REG sm750_sw_i2c_write_reg
> > +    #define I2C_READ_REG  sm750_sw_i2c_read_reg
> >  #endif
> >  
> >  /* SII164 Vendor and Device ID */
> > @@ -25,7 +25,7 @@
> >  
> >  #ifdef SII164_FULL_FUNCTIONS
> >  /* Name of the DVI Controller chip */
> > -static char *gDviCtrlChipName = "Silicon Image SiI 164";
> > +static char *dvi_controller_chip_name = "Silicon Image SiI 164";
> 
> This is a totally different thing.

It is, however I believe it is somewhat more descriptive, I suppose it
doesn't really matter though and if it should be the same, just made
snake_case, I can do that.
> 
> >  #endif
> >  
> >  /*
> > @@ -37,14 +37,14 @@ static char *gDviCtrlChipName = "Silicon Image
> > SiI 164";
> >   */
> >  unsigned short sii164_get_vendor_id(void)
> >  {
> > -	unsigned short vendorID;
> > +	unsigned short vendor;
> 
> Why change this?

Again removing camelCase, kernel style guide says that shorter names
are preferred (unless I misinterpreted that), I could make it vendor_id
if that is preferred, I believe the same would be with device_id below
it.

> This is a mix of lots of different changes, please break things up
> into
> "one logical change per patch"

Would it be best to split each rename (function or variable) into a
separate patch? I do agree it is quite a lot and I was a little unsure
when sending this one, but I also don't want to make a lot of different
patches and spam your email with 100 patches at once.

I suppose I could make one rename per patch and do them a few at a
time, that may take longer though.
> 
> thanks,
> 
> greg k-h