RE: [net-next-2.6 25/27] e1000e: static analysis tools complain of a possible null ptr p dereference

[Joe Perches <joe@perches.com>]

On Fri, 2010-12-10 at 12:35 -0700, Tantilov, Emil S wrote:
> >On Fri, 2010-12-10 at 02:06 -0800, Jeff Kirsher wrote:
> >> diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
> >[]
> >> +		default:
> >> +			data[i] = 0;
> >> +			continue;
> >> +			break;
> >Using
> >	continue;
> >	break;
> >is odd and unhelpful.
> >Just continue; is sufficient and clear.
> It's odd and without consequence but not necessarily "unhelpful" as it
> can protect from bugs in case someone was to add another case
> statement.

continue statements are not fall-through.

Adding another case statement in the switch below
this case label would work just fine.

> While unlikely, bugs in switch statements due to missing breaks are
> not unheard of.

True, but that's not this case.

> Looking at the kernel source there is no consistency as far as break
> in the default: case is concerned.

It's not the break, it's the break after continue that's odd.

Glancing through the source code using
	$ grep -rP --include=*.[ch] -B 3 "\bcontinue\s*;\s*break\s*;" *
this is a pretty unusual coding style.

Most all of the matches are like:
	if (condition)
		continue;
	break;

Your choice, your code.  I just think it's ugly
as well as odd and unhelpful.

cheers, Joe