[i2c] [PATCHv2 3/3] Add platform driver on top of the new pca-algorithm

[Trent Piepho]

On Sun, 16 Mar 2008, Jean Delvare wrote:
> On Sun, 16 Mar 2008 15:28:34 +0100, Wolfram Sang wrote:
> > On Fri, Mar 14, 2008 at 07:46:28PM +0100, Jean Delvare wrote:
> >
> > > > +	/* Use gpio_is_valid() when in mainline */
> > > > +	if (i2c->gpio > -1)
> > > > +		ret = gpio_request(i2c->gpio, i2c->adap.name);
> > > > +		if (ret == 0) {
> > > > +			gpio_direction_output(i2c->gpio, 1);
> > > > +			i2c->algo_data.reset_chip = i2c_pca_pf_resetchip;
> > > > +		} else {
> > > > +			printk(KERN_WARNING "%s: Registering gpio failed!\n",
> > > > +				i2c->adap.name);
> > > > +			i2c->gpio = ret;
> > > > +		}
> > >
> > > You're missing curly braces around this block, aren't you?
> > Holy ..., yes, you are right. This is why I used to have braces even
> > around single instructions after if. This is, where CodingStyle gives me
> > most trouble.
>
> I can't agree more... This type of warning I am carefully ignoring when
> checkpatch.pl complains. In many cases, the extra braces don't hurt and
> protect us against future mistakes.

I've been bit by this exact same problem enough times that I've come to the
same conclusion.  Adding a second statement to a one-statement if and then
forgetting to add the brances almost always produces code that compiles
without warnings and almost works.  It ends up being very hard to track
down, and even when you do, it's easy to stare at right at the bug and not
see the missing braces.