netdev - Re: [PATCH 2/5] dccp: Implement lookup table for feature-negotiation information

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]

Message-ID: <20080924135822.GI9588@ghostprotocols.net>
Date:	Wed, 24 Sep 2008 10:58:22 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Gerrit Renker <gerrit@....abdn.ac.uk>, davem@...emloft.net,
	dccp@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 2/5] dccp: Implement lookup table for
	feature-negotiation information

Em Wed, Sep 24, 2008 at 06:41:20AM +0200, Gerrit Renker escreveu:
> | > | > --- a/include/linux/dccp.h
> | > | > +++ b/include/linux/dccp.h
> | > | >  
> | > | > +
> | > | > +static int dccp_feat_default_value(u8 feat_num)
> | > | > +{
> | > | > +	int idx = dccp_feat_index(feat_num);
> | > | > +
> | > | > +	return idx < 0 ? : dccp_feat_table[idx].default_value;
> | > | > +}
> <snip>
> | > | 
> | > It is the first value. The test is only there to avoid accessing the
> | > array with an invalid index, which would happen if an unknown `feat_num'
> | > is passed - as for unknown features there is no default value.
> | 
> | The above explanation would be good to have as a comment, as it was not
> | so obvious from a first sight. I think that even having it explicit
> | would be clearer:
> | 
> | return idx < 0 ? 1 : dccp_feat_table[idx].default_value;
> | 
> | But then, if an unknown feat num is passed shouldn't the code bailout in
> | some other fashion than returning the result of a boolean expression and
> | not accessing the defaults table?
> | 
> Yes thank you. It is necessary to check this, since only in the current
> state of code the use of the function is consistent.  If the code gets
> changed later on then there will be no warning.
> 
> I have worked on this function yesterday evening with regard to above
> feedback. If you could either check again when the patch is submitted
> later, or have a look at the online version in the test tree on
> http://eden-feed.erg.abdn.ac.uk/cgi-bin/gitweb.cgi?p=dccp_exp.git;a=commitdiff;h=b0708121bfeb309db88e1b3a97cf851069bcafe1

Looks ok now, thanks.

- Arnaldo
--
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