On Thu, 25 Oct 2012 22:12:32 +0800 Richard Yang <weiyang@xxxxxxxxxxxxxxxxxx> wrote: > >drivers/scsi/libiscsi.c: In function 'iscsi_free_task': > >drivers/scsi/libiscsi.c:507:2: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi.c: In function 'iscsi_pool_init': > >drivers/scsi/libiscsi.c:2510:3: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi.c: In function 'iscsi_conn_setup': > >drivers/scsi/libiscsi.c:2881:2: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi.c: In function 'iscsi_conn_teardown': > >drivers/scsi/libiscsi.c:2944:2: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_cleanup_task': > >drivers/scsi/libiscsi_tcp.c:462:3: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c:469:3: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_r2t_rsp': > >drivers/scsi/libiscsi_tcp.c:570:3: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c:586:3: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c:596:2: warning: comparison of distinct pointer types lacks a cast [enabled by default] > >drivers/scsi/libiscsi_tcp.c: In function 'iscsi_tcp_get_curr_r2t': > >drivers/scsi/libiscsi_tcp.c:998:5: warning: comparison of distinct pointer types lacks a cast [enabled by default] > > > > ... > > I did a quick check. > > The root reason is, > 1. kfifo_in() second parameter should be type "const void *" > 2. kfifo_out_locked() second parameter should be type "void *" > 3. kfifo_in_locked() second parameter should be type "const void *" > > And I am curious about why the original code couldn't detect this type > mismatch. Well, just looking at __cxio_init_resource_fifo(): u32 entry = 0; ... kfifo_in(fifo, (unsigned char *) &entry, sizeof(u32)); This is silly - we shouldn't warn about this code. There is no reason for the kfifo library code to require that the *caller*'s storage be const! kfifo_in() is (or should be) simulating a C function with the interface: unsigned int kfifo_in(struct kfifo *fifo, const void *buf, size_t n); This states "this function does not modify the memory at *buf" and it's perfectly OK for a caller to pass a non-const pointer into kfifo_in(). We shouldn't warn about this. In fact, the __cxio_init_resource_fifo() shouldn't need to cast `buf' to uchar* either. The need to add that cast is a shortcoming in the existing kfifo code. We should be able to pass *any* pointer, of any type, const or non-const into code which expects a const void *, without any warning. And holy cow that code is hard to read :( Why was kfifo_in() implemented as a macro, anyway? AFAICT all its args have a known type, so we could have used a proper C interface, which would have fixed all this nicely. Anyway, include-linux-kfifoh-replace-open-coded-type-check-code-with-typecheck.patch takes the kfifo typechecking from "too strict" to "far too strict", so I'll drop that patch. -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
