On Tue, May 14, 2013 at 02:01:50PM +0800, Chen Gang wrote: > > HCF_MAX_NAME_LEN is 32, which may less than ''probe_rsp->rawData[1]'', > so need check the length when copy to ssid. > > Type of 'probe_rsp->rawData[1]' is 'hcf_8' which is 'unsigned char', > better to cast to 'unsigned int' firstly, and then cast to 'int'. Why not cast directly to int? It's the same in the end. > > For compiling, 'min()' is defined in include/linux/kernel.h, it has > '(void) (&_min1 == &_min2);', so if no type cast, the compiler will > report 'pointer types lacks a cast': > > drivers/staging/wlags49_h2/wl_main.c:3174:122: warning: comparison of distinct pointer types lacks a cast [enabled by default] > > > Signed-off-by: Chen Gang <gang.chen@xxxxxxxxxxx> > --- There should be something here under the --- line describing why v2 is needed like this: v2: add a cast to silence a compile warning This isn't the right way to do this. Use min_t(). + min_t(u8, + probe_rsp->rawData[1], + HCF_MAX_NAME_LEN - 1)); The reason I chose u8 here is because I feel it tells the reviewer that probe_rsp->rawData[1] is a u8. I prefer to cast to unsigned values so that reviewers know that a large positive value will not be cast to negative and counted as the minimum. The other bug that people do with min_t() casting is when you have something like this: unsigned int val = ... u8 x = min_t(u8, val, 10); In this case, the cast truncates high values of "val" so we use a small positive value but not 10 which was intended. In other words, the programmer cast it to u8 because "x" is a u8 but the type of "x" doesn't matter. It's "val" which matters and "val" is an unsigned int. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
