On Monday, January 28, 2013 09:04:29 PM Lan Tianyu wrote: > 于 2013/1/28 20:18, Rafael J. Wysocki 写道: > > On Monday, January 28, 2013 09:03:28 AM James Courtier-Dutton wrote: > >> On 28 January 2013 08:31, Lan Tianyu <tianyu.lan@xxxxxxxxx> wrote: > >>> - if (len == sizeof ctrl_auto - 1 && strncmp(buf, ctrl_auto, len) == 0) > >>> + if (len == sizeof(ctrl_auto - 1) && strncmp(buf, ctrl_auto, len) == 0) > >> > >> This looks wrong to me. > >> sizeof ctrl_auto - 1 > >> is not the same value as > >> sizeof(ctrl_auto - 1) > >> because sizeof(x) is normally the same as sizeof(x - 1), unless sizeof > >> x and sizeof 1 are different. > >> Consider that is maybe should be: > >> if (len == (sizeof(ctrl_auto) - 1)) && strncmp(buf, ctrl_auto, len) == 0) > Hi James: > Yes. You are correct. Thanks for your review. > > > > The outer parentheses in the comparison with len are not necessary. > > > >> I a not sure what the correct answer is for this particular bit of > >> code, because I have not looked at it in detail,I just wanted to point > >> out that the brackets might be in the wrong place here. > > > > You are right and the patch doesn't make sense. > Hi Rafael: > So this patch is not necessary? It is incorrect in the first place. Yes, you could change spaces to parentheses in those places, but first, please do that correctly and second, it's just going to be a cosmetic change. The code works as is. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html