Add compat_ioctl to st
Subsystems (2)
PATCH 1/1 Add compat_ioctl to st
Call new compat_ioctl host vector from tape driver
diff -u linux-2.6.11-rc1-bk4/drivers/scsi/scsi_ioctl.c-o linux-2.6.11-rc1-bk4/drivers/scsi/scsi_ioctl.c
drivers/scsi/st.c renamed from drivers/scsi/st.c-o +16 -0
@@ -3425,6 +3425,22 @@up (&STp->lock);return retval; } + +#ifdef CONFIG_COMPAT +static int st_compat_ioctl (struct file *file,unsigned int cmd,unsigned long arg) +{ +struct scsi_tape *STp = file->private_data; +struct scsi_device *sdev = STp->device; +int ret = -ENOIOCTLCMD ; +if (sdev->host->hostt->compat_ioctl) { + + ret = sdev->host->hostt->compat_ioctl (sdev, cmd, arg); + + } +return ret; +} +#endif +
/* Try to allocate a new tape buffer. Calling function must not hold
@@ -3716,6 +3732,9 @@
.read = st_read,
.write = st_write,
.ioctl = st_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = st_compat_ioctl,
+#endif
.open = st_open,
.flush = st_flush,
.release = st_release,
drivers/scsi/st.c line 3437
Quoted 9 lines
drivers/scsi/st.c renamed from drivers/scsi/st.c-o +9
-0+#ifdef CONFIG_COMPAT +static int st_compat_ioctl (struct file *file,unsigned int cmd,unsigned long arg) +{ +struct scsi_tape *STp = file->private_data; +struct scsi_device *sdev = STp->device; +int ret = -ENOIOCTLCMD ; +if (sdev->host->hostt->compat_ioctl) { + + ret = sdev->host->hostt->compat_ioctl (sdev, cmd, arg);
This seems to miss a cast for arg. Also I think it would be really nice to have a little helper like:
int scsi_compat_ioctl(struct scsi_device *sdev, unsigned int cmd,
unsiged long arg)
{
struct scsi_host_template *hostt = sdev->host->hostt;
if (hostt->compat_ioctl)
return hostt->compat_ioctl(sdev, cmd, (void __user *)arg);
return -ENOIOCTLCMD;
}that all drivers could use.
Quoted 4 lines · Christoph Hellwig
Quoted 4 lines
drivers/scsi/st.c renamed from drivers/scsi/st.c-o +4
-0+#ifdef CONFIG_COMPAT +static int st_compat_ioctl (struct file *file,unsigned int cmd,unsigned long arg) +{ +struct scsi_tape *STp = file->private_data;
Quoted 8 lines
drivers/scsi/st.c line 3437
Quoted 5 lines
drivers/scsi/st.c renamed from drivers/scsi/st.c-o +5
-0+struct scsi_device *sdev = STp->device; +int ret = -ENOIOCTLCMD ; +if (sdev->host->hostt->compat_ioctl) { + + ret = sdev->host->hostt->compat_ioctl (sdev, cmd, arg);This seems to miss a cast for arg. Also I think it would be really nice to have a little helper like:
Hmm, yes.
Well, there are so many warnings in the code currently anyways that one more won't really hurt ;-)
Quoted 11 lines
int scsi_compat_ioctl(struct
scsi_device*sdev, unsigned int cmd,
unsiged long arg){ struct scsi_host_template *hostt = sdev->host->hostt; if (hostt->compat_ioctl) return hostt->compat_ioctl(sdev, cmd, (void __user *)arg); return -ENOIOCTLCMD; }that all drivers could use.
I don't think this makes sense for three lines of code.
-Andi
Review line 3740
Quoted 10 lines · Andi Kleen
(quoted diff) +3
-0@@ -3716,6 +3732,9 @@ .read = st_read, .write = st_write, .ioctl = st_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = st_compat_ioctl, +#endif .open = st_open, .flush = st_flush, .release = st_release,
How about having a macro ...
.ioctl = st_ioctl, COMPAT_IOCTL_ENTRY(st_compat_ioctl) .open = st_open,
which could be defined something like:
#ifdef CONFIG_COMPAT
#define COMPAT_IOCTL_ENTRY(x) .compat_ioctl = x,
#else
#define COMPAT_IOCTL_ENTRY(x) /* */
#endif
Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain
Quoted 6 lines
How about having a macro ...
.ioctl = st_ioctl, COMPAT_IOCTL_ENTRY(st_compat_ioctl) .open = st_open,
Quoted 7 lines
which could be defined something like:
#ifdef
CONFIG_COMPAT
#defineCOMPAT_IOCTL_ENTRY(x).compat_ioctl = x,
#else
#defineCOMPAT_IOCTL_ENTRY(x)/* */
#endif
To be honest I prefer the ifdef. It makes it clear what is going on, without any unneeded abstraction. The macro in comparison looks unnatural in the definition block.
-Andi
Quoted 14 lines · Andi Kleen
Quoted 6 lines
How about having a macro ...
.ioctl = st_ioctl, COMPAT_IOCTL_ENTRY(st_compat_ioctl) .open = st_open,Quoted 7 lines
which could be defined something like:
#ifdef
CONFIG_COMPAT
#defineCOMPAT_IOCTL_ENTRY(x).compat_ioctl = x,
#else
#defineCOMPAT_IOCTL_ENTRY(x)/* */
#endif
Quoted 4 lines
To be honest I prefer the ifdef. It makes it clear what is going on, without any unneeded abstraction. The macro in comparison looks unnatural in the definition block.
It would have prevented the mistake you made in sg (which message I only read after writing this). Another way of managing without ifdefs would be:
.compat_ioctl = COMPAT_IOCTL_ENTRY(st),
with
#ifdef CONFIG_COMPAT
#define COMPAT_IOCTL_ENTRY(x) x##_compat_ioctl
#else
#define COMPAT_IOCTL_ENTRY(x) NULL
#endif
Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain