Re: [bug report] leds: Add support for Philips PCA955x I2C LED drivers

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Aug 17, 2017 at 04:37:22PM +0300, Dan Carpenter wrote:
> On Thu, Aug 17, 2017 at 03:27:40PM +0200, Pavel Machek wrote:
> > On Thu 2017-08-17 14:51:48, Dan Carpenter wrote:
> > > On Thu, Aug 17, 2017 at 01:27:38PM +0200, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > > Hello Nate Case,
> > > > > 
> > > > > The patch f46e9203d9a1: "leds: Add support for Philips PCA955x I2C
> > > > > LED drivers" from Jul 16, 2008, leads to the following static checker
> > > > > warning:
> > 
> > > > >    472                          /*
> > > > >    473                           * Platform data can specify LED names and
> > > > >    474                           * default triggers
> > > > >    475                           */
> > > > >    476                          if (pdata->leds[i].name)
> > > > >                                     ^^^^^^^^^^^^^^^^^^^
> > > > > The comment implies that we should be testing pdata->leds[i].name[0] to
> > > > > see if any string has been set?
> > > > 
> > > > Someone was already submitting patch from this one, no?
> > > > 
> > > > And please don't mark this one as a "bug report". There's no bug. Your
> > > > code analysis tool found a way to make kernel code shorter... well, so
> > > > what?
> > > > 
> > > 
> > > You didn't read my email.  It was only one sentence long...  :/
> > 
> > I read your subject, and it claimed bug.
> > 
> > Bug normally means: Hey, there's problem with the driver. It oopses my
> > kernel here.
> > 
> 
> I thought it probably was a bug.  One hour ago Colin "fixed" one of
> these by removing the NULL check and it turned out that it was a real
> bug.  It's legit to suspect that these are often bugs.
> 

It's especially legit to suspect that the check might be important if
there is a comment explaining why the check is there.  If you don't want
people to question your code stop writing nonsense code in the first
place.  Gar...

regards,
dan carpenter