| Message ID | 6309932.EjHf7uUo1X@wuerfel (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 19 May 2015 23:22:39 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > The recent change to mark the input argument of sg_pcopy_from_buffer > had the unfortunate side-effect to cause a new warning in the > scsi_debug code: > > drivers/scsi/scsi_debug.c: In function 'do_device_access': > drivers/scsi/scsi_debug.c:2376:8: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types] > func = sg_pcopy_from_buffer; > > This patch attempts to avoid that warning without adding > evil type casts, but unfortunately makes the do_device_access > function a lot uglier in the process. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 5250326459 ("lib/scatterlist: mark input buffer parameters as 'const'") > --- > > I can't decide if this is actually a good idea, or if we should rather drop > the sg_pcopy_from_buffer() patch. Maybe someone else sees a better solution. Could make do_device_access() call sg_copy_buffer() directly. But yes, dropping the sg_pcopy_from/to_buffer changes is reasonable. sg_copy_buffer() is bidirectional and that won't be changing, so putting constified wrapeprs around it is kinda fake. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 20 May 2015 12:53:29 Andrew Morton wrote: > On Tue, 19 May 2015 23:22:39 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > > > > I can't decide if this is actually a good idea, or if we should rather drop > > the sg_pcopy_from_buffer() patch. Maybe someone else sees a better solution. > > Could make do_device_access() call sg_copy_buffer() directly. > > But yes, dropping the sg_pcopy_from/to_buffer changes is reasonable. > sg_copy_buffer() is bidirectional and that won't be changing, so > putting constified wrapeprs around it is kinda fake. Ok. The part I only saw now is that do_device_access() is the only user of sg_pcopy_from_buffer(), so if that passes a non-const argument, there is dropping the patch will be teh best solution. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 1f8e2dc9c616..dd9f0d1574ef 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -2363,17 +2363,13 @@ do_device_access(struct scsi_cmnd *scmd, u64 lba, u32 num, bool do_write) u64 block, rest = 0; struct scsi_data_buffer *sdb; enum dma_data_direction dir; - size_t (*func)(struct scatterlist *, unsigned int, void *, size_t, - off_t); if (do_write) { sdb = scsi_out(scmd); dir = DMA_TO_DEVICE; - func = sg_pcopy_to_buffer; } else { sdb = scsi_in(scmd); dir = DMA_FROM_DEVICE; - func = sg_pcopy_from_buffer; } if (!sdb->length) @@ -2385,17 +2381,29 @@ do_device_access(struct scsi_cmnd *scmd, u64 lba, u32 num, bool do_write) if (block + num > sdebug_store_sectors) rest = block + num - sdebug_store_sectors; - ret = func(sdb->table.sgl, sdb->table.nents, - fake_storep + (block * scsi_debug_sector_size), - (num - rest) * scsi_debug_sector_size, 0); + if (do_write) + ret = sg_pcopy_to_buffer(sdb->table.sgl, sdb->table.nents, + fake_storep + (block * scsi_debug_sector_size), + (num - rest) * scsi_debug_sector_size, 0); + else + ret = sg_pcopy_from_buffer(sdb->table.sgl, sdb->table.nents, + fake_storep + (block * scsi_debug_sector_size), + (num - rest) * scsi_debug_sector_size, 0); + if (ret != (num - rest) * scsi_debug_sector_size) return ret; - if (rest) { - ret += func(sdb->table.sgl, sdb->table.nents, - fake_storep, rest * scsi_debug_sector_size, - (num - rest) * scsi_debug_sector_size); - } + if (!rest) + return ret; + + if (do_write) + ret += sg_pcopy_to_buffer(sdb->table.sgl, sdb->table.nents, + fake_storep, rest * scsi_debug_sector_size, + (num - rest) * scsi_debug_sector_size); + else + ret += sg_pcopy_from_buffer(sdb->table.sgl, sdb->table.nents, + fake_storep, rest * scsi_debug_sector_size, + (num - rest) * scsi_debug_sector_size); return ret; }
The recent change to mark the input argument of sg_pcopy_from_buffer had the unfortunate side-effect to cause a new warning in the scsi_debug code: drivers/scsi/scsi_debug.c: In function 'do_device_access': drivers/scsi/scsi_debug.c:2376:8: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types] func = sg_pcopy_from_buffer; This patch attempts to avoid that warning without adding evil type casts, but unfortunately makes the do_device_access function a lot uglier in the process. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 5250326459 ("lib/scatterlist: mark input buffer parameters as 'const'") --- I can't decide if this is actually a good idea, or if we should rather drop the sg_pcopy_from_buffer() patch. Maybe someone else sees a better solution. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html