netdev - Re: [net v6 1/8] i40e: main driver core

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 7 Sep 2013 19:27:53 +0000
From:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>
To:	Daniel Borkmann <dborkman@...hat.com>
CC:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>,
	"Nelson, Shannon" <shannon.nelson@...el.com>,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>
Subject: Re: [net v6 1/8] i40e: main driver core

On Sat, 2013-09-07 at 09:15 +0200, Daniel Borkmann wrote:

> Thanks for including SCTP! ;-)

:-)


> Here as well, I40E_ERR_NO_MEMORY vs -ENOMEM.
> 

okay we will audit those and fix them.

> 
> Nitpick: why s32 in the signature? There are a lot of such places, just int would have
> been fine probably.
> 

agreed.

> 			}
> > +			if (pf->lan_veb == I40E_NO_VEB) {
> > +				v = i40e_veb_mem_alloc(pf);
> > +				if (v < 0)
> > +					break;
> 
> Nitpick: I'd expect from *alloc() functions to return NULL, but fair enough.

I will take a look at what we can do about these.


> > +	aq_buf = kzalloc(I40E_AQ_LARGE_BUF, GFP_KERNEL);
> > +	if (!aq_buf)
> > +		return -ENOMEM;
> 
> More of such examples ... ;-) I40E_ERR_BAD_PTR vs. -ENOMEM on a s32 instead of int.

right

> > +	} else if (pf->flags & I40E_FLAG_RSS_ENABLED	  &&
> > +		   !(pf->flags & I40E_FLAG_FDIR_ENABLED)  &&
> > +		   !(pf->flags & I40E_FLAG_DCB_ENABLED)) {
> > +
> > +		SET_RSS_SIZE;
> 
> Can't these macros be done in a small inline function instead?

I'll take a look, as well for all instances of this.

Thanks again,
 Jesse