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 <[email protected]>
> ---

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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to