At Mon, 15 Dec 2014 02:01:18 +0900, Takashi Sakamoto wrote: > > On Dec 14 2014 02:30, Takashi Iwai wrote: > > At Sat, 13 Dec 2014 19:14:46 +0900, > > Takashi Sakamoto wrote: > >> > >> On Dec 13 2014 16:04, Dan Carpenter wrote: > >>> On Sat, Dec 13, 2014 at 02:06:59PM +0900, Takashi Sakamoto wrote: > >>>> On Dec 13 2014 04:27, Dan Carpenter wrote: > >>>>> This code tends to use unsigned variables by default and it causes > >>>>> signedness bugs when we use negative variables for error handling. > >>>>> The "i" and "j" variables are used to iterated over small positive > >>>>> values and so they should be type "int". The "len" variable doesn't > >>>>> *need* to be signed but it should be signed to make the code easier to > >>>>> read and audit. > >>>>> > >>>>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > > > Note that Dan's suggestions are about the variables like "i" and "j". > > These are used normally for small loop counts, and they are int in a > > standard idiom. If they are declared as another type, you force > > reader's attention *unnecessarily*, and it decreases the readability > > (i.e. they have to read the loop code as somewhat special even if > > it's a normal loop). This is the only big drawback, and the rest > > merit/demerit are almost ignorable, IMO. > > > > Of course, in some cases, a loop count might be better in unsigned. > > But then a different variable name should be used instead. > > > > After all, this is rather a minor issue, almost a bikeshed topic, so I > > didn't care much while reviewing your patches, and I still don't care > > whether this fix patch will have int or unsigned for i. But, it'd be > > good if you keep this information in your mind, at least. > > Reviewed-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> OK, merged now. Thanks. Takashi -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
