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: <20080924044120.GA4944@gerrit.erg.abdn.ac.uk>
Date:	Wed, 24 Sep 2008 06:41:20 +0200
From:	Gerrit Renker <gerrit@....abdn.ac.uk>
To:	Arnaldo Carvalho de Melo <acme@...hat.com>, davem@...emloft.net,
	dccp@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 2/5] dccp: Implement lookup table for
	feature-negotiation information

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

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