Please read my first email again. Pretty much everything I wanted to say is there. http://driverdev.linuxdriverproject.org/pipermail/devel/2013-May/038083.html On Wed, May 15, 2013 at 06:09:14PM +0800, Chen Gang wrote: > If the type of 'probe_rsp->rawData[]' is changed to 'char', it will > compare between 'char' and 'int', in this condition, casting to 'int' is > the normal way, all things are OK. You mean "unsigned char" instead of "char"... Signed char would be a bug. > > If use min_t(u8..., we have to look up the 'HCF_MAX_NAME_LEN' whether is > changed upper than 255. No. 1) I would assume the programmer had considered that. 2) That kind of a bug would almost certainly be caught in testing. 3) How many values with a name like HCF_MAX_NAME_LEN are higher than 255? Names are normally are 16-32 characters long. When I am reviewing and I see min_t(): 1) I assume that the two types are different and that the compiler warned about it. 2) I assume that HCF_MAX_NAME_LEN is an int so "HCF_MAX_NAME_LEN - 1" is also an int. When I see "min_t(int, XXX, HCF_MAX_NAME_LEN - 1)" then I think: 1) I know that "HCF_MAX_NAME_LEN - 1" is int, but I don't know the type for "XXX". 2) Are negative values for XXX valid? 3) Did the author stop to consider that high values can be cast to negative? 4) A lot of people just use "int" without even considering if it's appropriate. For example, here is a real bug I fixed last week: http://marc.info/?l=linux-kernel&m=136845723902257&w=2 This bug can only be triggered by root but it was still a real bug where "int" was used instead of "unsigned int". If I see that the type is cast using "min_t(u8," then I will assume that "XXX" is a u8. It is also possible that the author chose "min_t(u8," when neither side of the comparison is a u8. The common reason for that is because they are assigning the result to an u8 variable. But from the immediate context I would see that that is not the case here. But again, in your specific cast casting it to an int is fine. It doesn't cause a bug. It only takes a little extra time for a reviewer. Whatever... It's up to you. Send any patch which applies and has the proper signed-off-by and I will ack it. :P regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
