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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


| > | > --- 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 dccp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Linux Kernel]     [IETF DCCP]     [Linux Networking]     [Git]     [Security]     [Linux Assembly]     [Bugtraq]     [Photo]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Linux Resources]

Powered by Linux