Re: [PATCH] tty add compat_ioctl method
On Thursday 03 May 2007, Paul Fulghum wrote: > > > declarations should never be hidden inside of an #ifdef. If you want to be > > extra clever here, you can do > > OK, I have no problem with that. > A declaration without implementation won't generate a warning? You only get a warning for static declarations without an implementation, not for globals. Arnd <>< - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tty add compat_ioctl method
Arnd Bergmann wrote: Looks ok mostly. Just some details: ... +#ifdef CONFIG_COMPAT + long (*compat_ioctl)(struct tty_struct *tty, struct file * file, +unsigned int cmd, unsigned long arg); +#endif I wouldn't hide this inside of an #ifdef. The structures are all static and therefore a single field per driver doesn't add much bload, but being able to always assign the .compat_ioctl pointer makes the code somewhat nicer OK --- a/drivers/char/tty_io.c 2006-11-29 15:57:37.0 -0600 +++ b/drivers/char/tty_io.c 2007-04-30 14:51:01.0 -0500 @@ -151,6 +151,9 @@ static int tty_open(struct inode *, stru static int tty_release(struct inode *, struct file *); int tty_ioctl(struct inode * inode, struct file * file, unsigned int cmd, unsigned long arg); +#ifdef CONFIG_COMPAT +long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg); +#endif static int tty_fasync(int fd, struct file * filp, int on); static void release_mem(struct tty_struct *tty, int idx); declarations should never be hidden inside of an #ifdef. If you want to be extra clever here, you can do OK, I have no problem with that. A declaration without implementation won't generate a warning? #ifdef CONFIG_COMPAT long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg); #else #define tty_compat_ioctl NULL #endif then you don't need an #ifdef in the code setting the function pointers. OK + tty = (struct tty_struct *)file->private_data; no need for the cast, ->private_data is void*. Yes, an easy fix. -- Paul - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tty add compat_ioctl method
On Wednesday 02 May 2007, Paul Fulghum wrote:
> Add compat_ioctl method for tty code to allow processing
> of 32 bit ioctl calls on 64 bit systems by tty core,
> tty drivers, and line disciplines.
Looks ok mostly. Just some details:
> --- a/include/linux/tty_driver.h 2006-11-29 15:57:37.0 -0600
> +++ b/include/linux/tty_driver.h 2007-04-30 14:05:02.0 -0500
> @@ -132,6 +132,10 @@ struct tty_operations {
> int (*chars_in_buffer)(struct tty_struct *tty);
> int (*ioctl)(struct tty_struct *tty, struct file * file,
> unsigned int cmd, unsigned long arg);
> +#ifdef CONFIG_COMPAT
> + long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
> + unsigned int cmd, unsigned long arg);
> +#endif
> void (*set_termios)(struct tty_struct *tty, struct termios * old);
> void (*throttle)(struct tty_struct * tty);
> void (*unthrottle)(struct tty_struct * tty);
> @@ -193,6 +197,10 @@ struct tty_driver {
> int (*chars_in_buffer)(struct tty_struct *tty);
> int (*ioctl)(struct tty_struct *tty, struct file * file,
> unsigned int cmd, unsigned long arg);
> +#ifdef CONFIG_COMPAT
> + long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
> + unsigned int cmd, unsigned long arg);
> +#endif
> void (*set_termios)(struct tty_struct *tty, struct termios * old);
> void (*throttle)(struct tty_struct * tty);
> void (*unthrottle)(struct tty_struct * tty);
I wouldn't hide this inside of an #ifdef. The structures are all static
and therefore a single field per driver doesn't add much bload, but
being able to always assign the .compat_ioctl pointer makes the code
somewhat nicer
> --- a/drivers/char/tty_io.c 2006-11-29 15:57:37.0 -0600
> +++ b/drivers/char/tty_io.c 2007-04-30 14:51:01.0 -0500
> @@ -151,6 +151,9 @@ static int tty_open(struct inode *, stru
> static int tty_release(struct inode *, struct file *);
> int tty_ioctl(struct inode * inode, struct file * file,
> unsigned int cmd, unsigned long arg);
> +#ifdef CONFIG_COMPAT
> +long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long
> arg);
> +#endif
> static int tty_fasync(int fd, struct file * filp, int on);
> static void release_mem(struct tty_struct *tty, int idx);
declarations should never be hidden inside of an #ifdef. If you want to be
extra clever here, you can do
#ifdef CONFIG_COMPAT
long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg);
#else
#define tty_compat_ioctl NULL
#endif
then you don't need an #ifdef in the code setting the function pointers.
> @@ -3284,6 +3299,32 @@ int tty_ioctl(struct inode * inode, stru
> return retval;
> }
>
> +#ifdef CONFIG_COMPAT
> +long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long
> arg)
> +{
> + struct inode *inode = file->f_dentry->d_inode;
> + struct tty_struct *tty;
> + struct tty_ldisc *ld;
> + int retval = -ENOIOCTLCMD;
> +
> + tty = (struct tty_struct *)file->private_data;
no need for the cast, ->private_data is void*.
Arnd <><
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] tty add compat_ioctl method
Add compat_ioctl method for tty code to allow processing
of 32 bit ioctl calls on 64 bit systems by tty core,
tty drivers, and line disciplines.
Based on patch by Arnd Bergmann:
http://www.uwsg.iu.edu/hypermail/linux/kernel/0511.0/1732.html
This patch does not remove tty ioctl entries in compat_ioctl.[ch]
That will be implemented in a separate patch.
Signed-off-by: Paul Fulghum <[EMAIL PROTECTED]>
CC: Arnd Bergmann <[EMAIL PROTECTED]>
--- a/drivers/char/n_tty.c 2007-04-30 15:13:19.0 -0500
+++ b/drivers/char/n_tty.c 2007-04-30 15:17:28.0 -0500
@@ -1545,21 +1545,18 @@ static unsigned int normal_poll(struct t
}
struct tty_ldisc tty_ldisc_N_TTY = {
- TTY_LDISC_MAGIC,/* magic */
- "n_tty",/* name */
- 0, /* num */
- 0, /* flags */
- n_tty_open, /* open */
- n_tty_close,/* close */
- n_tty_flush_buffer, /* flush_buffer */
- n_tty_chars_in_buffer, /* chars_in_buffer */
- read_chan, /* read */
- write_chan, /* write */
- n_tty_ioctl,/* ioctl */
- n_tty_set_termios, /* set_termios */
- normal_poll,/* poll */
- NULL, /* hangup */
- n_tty_receive_buf, /* receive_buf */
- n_tty_write_wakeup /* write_wakeup */
+ .magic = TTY_LDISC_MAGIC,
+ .name= "n_tty",
+ .open= n_tty_open,
+ .close = n_tty_close,
+ .flush_buffer= n_tty_flush_buffer,
+ .chars_in_buffer = n_tty_chars_in_buffer,
+ .read= read_chan,
+ .write = write_chan,
+ .ioctl = n_tty_ioctl,
+ .set_termios = n_tty_set_termios,
+ .poll= normal_poll,
+ .receive_buf = n_tty_receive_buf,
+ .write_wakeup= n_tty_write_wakeup
};
--- a/include/linux/tty_driver.h2006-11-29 15:57:37.0 -0600
+++ b/include/linux/tty_driver.h2007-04-30 14:05:02.0 -0500
@@ -132,6 +132,10 @@ struct tty_operations {
int (*chars_in_buffer)(struct tty_struct *tty);
int (*ioctl)(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+ long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
+unsigned int cmd, unsigned long arg);
+#endif
void (*set_termios)(struct tty_struct *tty, struct termios * old);
void (*throttle)(struct tty_struct * tty);
void (*unthrottle)(struct tty_struct * tty);
@@ -193,6 +197,10 @@ struct tty_driver {
int (*chars_in_buffer)(struct tty_struct *tty);
int (*ioctl)(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+ long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
+unsigned int cmd, unsigned long arg);
+#endif
void (*set_termios)(struct tty_struct *tty, struct termios * old);
void (*throttle)(struct tty_struct * tty);
void (*unthrottle)(struct tty_struct * tty);
--- a/drivers/char/tty_io.c 2006-11-29 15:57:37.0 -0600
+++ b/drivers/char/tty_io.c 2007-04-30 14:51:01.0 -0500
@@ -151,6 +151,9 @@ static int tty_open(struct inode *, stru
static int tty_release(struct inode *, struct file *);
int tty_ioctl(struct inode * inode, struct file * file,
unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg);
+#endif
static int tty_fasync(int fd, struct file * filp, int on);
static void release_mem(struct tty_struct *tty, int idx);
@@ -1153,8 +1156,8 @@ static unsigned int hung_up_tty_poll(str
return POLLIN | POLLOUT | POLLERR | POLLHUP | POLLRDNORM | POLLWRNORM;
}
-static int hung_up_tty_ioctl(struct inode * inode, struct file * file,
-unsigned int cmd, unsigned long arg)
+static long hung_up_tty_ioctl(struct file * file,
+ unsigned int cmd, unsigned long arg)
{
return cmd == TIOCSPGRP ? -ENOTTY : -EIO;
}
@@ -1165,6 +1168,9 @@ static const struct file_operations tty_
.write = tty_write,
.poll = tty_poll,
.ioctl = tty_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = tty_compat_ioctl,
+#endif
.open = tty_open,
.release= tty_release,
.fasync = tty_fasync,
@@ -1177,6 +1183,9 @@ static const struct file_operations ptmx
.write = tty_write,
.poll = tty_poll,
.ioctl = tty_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = tty_compat_ioctl,
+#endif
.open = ptmx_open,
.release=
