On Thu, Dec 12, 2019 at 5:25 PM Christoph Hellwig hch@infradead.org wrote:
On Wed, Dec 11, 2019 at 09:42:36PM +0100, Arnd Bergmann wrote:
--- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -198,6 +198,7 @@ static void sg_device_destroy(struct kref *kref);
#define SZ_SG_HEADER sizeof(struct sg_header) #define SZ_SG_IO_HDR sizeof(sg_io_hdr_t) +#define SZ_COMPAT_SG_IO_HDR sizeof(struct compat_sg_io_hdr)
I'd rather not add more defines like this. The raw sizeof is much more readable and obvious.
Done. I actually had it that way in the previous submission and then changed it for consistency. I considered removing SZ_SG_IO_HDR as well, but decided not to make Doug's life harder than necessary -- he has nother 50 or so patches on top of this that he needs to rebase.
I find the structure here a little confusing, as it doesn't follow the normal flow. What do you think of:
if (count >= SZ_SG_HEADER) { if (get_user(reply_len, &old_hdr->reply_len)) return -EFAULT;
I don't see much benefit either way. Changed it now it as you suggested.
Thanks for the review!
Arnd