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

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]

Date:	Fri, 10 Dec 2010 11:58:31 -0800
From:	Joe Perches <joe@...ches.com>
To:	"Tantilov, Emil S" <emil.s.tantilov@...el.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"Allan, Bruce W" <bruce.w.allan@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"bphilips@...ell.com" <bphilips@...ell.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"bphilips@...ell.com" <bphilips@...ell.com>
Subject: RE: [net-next-2.6 25/27] e1000e: static analysis tools complain of
 a possible null ptr p dereference

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html