| Message ID | 20170603135111.5444-2-asarai@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
> diff --git a/Makefile b/Makefile > index 470bd4d9513a..fb689286d83a 100644 > --- a/Makefile > +++ b/Makefile > @@ -401,6 +401,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ > -fno-strict-aliasing -fno-common \ > -Werror-implicit-function-declaration \ > -Wno-format-security \ > + -Wno-error=int-in-bool-context \ > -std=gnu89 $(call cc-option,-fno-PIE) Ah sorry, I committed that by accident. I'll send a fixed version.
Hi Aleksa,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Aleksa-Sarai/tty-add-TIOCGPTPEER-ioctl/20170603-220322
config: i386-randconfig-x018-201722 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
>> cc1: warning: -malign-functions is obsolete, use -falign-functions
>> cc1: warning: -malign-jumps is obsolete, use -falign-jumps
>> cc1: warning: -malign-loops is obsolete, use -falign-loops
cc1: error: -Werror=int-in-bool-context: no option -Wint-in-bool-context
kernel/bounds.c:1:0: error: CPU you selected does not support x86-64 instruction set
/*
kernel/bounds.c:1:0: error: CPU you selected does not support x86-64 instruction set
kernel/bounds.c:1:0: warning: -mregparm is ignored in 64-bit mode
make[2]: *** [kernel/bounds.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Aleksa,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Aleksa-Sarai/tty-add-TIOCGPTPEER-ioctl/20170603-220322
config: x86_64-randconfig-x010-201722 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> cc1: error: -Werror=int-in-bool-context: no option -Wint-in-bool-context
make[2]: *** [kernel/bounds.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, Jun 3, 2017 at 3:51 PM, Aleksa Sarai <asarai@suse.de> wrote: > In order to avoid future diversions between fs/compat_ioctl.c and > drivers/tty/pty.c, define .compat_ioctl callbacks for the relevant > tty_operations structs. Since both pty_unix98_ioctl() and > pty_bsd_ioctl() are compatible between 32-bit and 64-bit userspace no > special translation is required. > > Signed-off-by: Aleksa Sarai <asarai@suse.de> > --- > Makefile | 1 + > drivers/tty/pty.c | 22 ++++++++++++++++++++++ > fs/compat_ioctl.c | 6 ------ > 3 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/Makefile b/Makefile > index 470bd4d9513a..fb689286d83a 100644 > --- a/Makefile > +++ b/Makefile > @@ -401,6 +401,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ > -fno-strict-aliasing -fno-common \ > -Werror-implicit-function-declaration \ > -Wno-format-security \ > + -Wno-error=int-in-bool-context \ > -std=gnu89 $(call cc-option,-fno-PIE) This slipped in by accident I assume? It seems completely unrelated. > diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c > index 65799575c666..2a6bd9ae3f8b 100644 > --- a/drivers/tty/pty.c > +++ b/drivers/tty/pty.c > @@ -481,6 +481,16 @@ static int pty_bsd_ioctl(struct tty_struct *tty, > return -ENOIOCTLCMD; > } > > +static long pty_bsd_compat_ioctl(struct tty_struct *tty, > + unsigned int cmd, unsigned long arg) > +{ > + /* > + * PTY ioctls don't require any special translation between 32-bit and > + * 64-bit userspace, they are already compatible. > + */ > + return pty_bsd_ioctl(tty, cmd, arg); > +} > + This looks correct but unnecessary, you can simply point both function pointers to the same function: > * not really modular, but the easiest way to keep compat with existing > @@ -502,6 +512,7 @@ static const struct tty_operations master_pty_ops_bsd = { > .chars_in_buffer = pty_chars_in_buffer, > .unthrottle = pty_unthrottle, > .ioctl = pty_bsd_ioctl, > + .compat_ioctl = pty_bsd_compat_ioctl, .compat_ioctl = pty_bsd_ioctl, The separate handler would only be required when you need any kind of special handling depending on the command. > diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c > index 6116d5275a3e..112b3e1e20e3 100644 > --- a/fs/compat_ioctl.c > +++ b/fs/compat_ioctl.c > @@ -866,8 +866,6 @@ COMPATIBLE_IOCTL(TIOCGDEV) > COMPATIBLE_IOCTL(TIOCCBRK) > COMPATIBLE_IOCTL(TIOCGSID) > COMPATIBLE_IOCTL(TIOCGICOUNT) > -COMPATIBLE_IOCTL(TIOCGPKT) > -COMPATIBLE_IOCTL(TIOCGPTLCK) > COMPATIBLE_IOCTL(TIOCGEXCL) > /* Little t */ > COMPATIBLE_IOCTL(TIOCGETD) > @@ -883,16 +881,12 @@ COMPATIBLE_IOCTL(TIOCMGET) > COMPATIBLE_IOCTL(TIOCMBIC) > COMPATIBLE_IOCTL(TIOCMBIS) > COMPATIBLE_IOCTL(TIOCMSET) > -COMPATIBLE_IOCTL(TIOCPKT) > COMPATIBLE_IOCTL(TIOCNOTTY) > COMPATIBLE_IOCTL(TIOCSTI) > COMPATIBLE_IOCTL(TIOCOUTQ) > COMPATIBLE_IOCTL(TIOCSPGRP) > COMPATIBLE_IOCTL(TIOCGPGRP) > -COMPATIBLE_IOCTL(TIOCGPTN) > -COMPATIBLE_IOCTL(TIOCSPTLCK) > COMPATIBLE_IOCTL(TIOCSERGETLSR) > -COMPATIBLE_IOCTL(TIOCSIG) Looks good. Arnd
>> diff --git a/Makefile b/Makefile >> index 470bd4d9513a..fb689286d83a 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -401,6 +401,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ >> -fno-strict-aliasing -fno-common \ >> -Werror-implicit-function-declaration \ >> -Wno-format-security \ >> + -Wno-error=int-in-bool-context \ >> -std=gnu89 $(call cc-option,-fno-PIE) > > This slipped in by accident I assume? It seems completely unrelated. Yeah, I re-sent v4 with this removed immediately afterwards. > >> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c >> index 65799575c666..2a6bd9ae3f8b 100644 >> --- a/drivers/tty/pty.c >> +++ b/drivers/tty/pty.c >> @@ -481,6 +481,16 @@ static int pty_bsd_ioctl(struct tty_struct *tty, >> return -ENOIOCTLCMD; >> } >> >> +static long pty_bsd_compat_ioctl(struct tty_struct *tty, >> + unsigned int cmd, unsigned long arg) >> +{ >> + /* >> + * PTY ioctls don't require any special translation between 32-bit and >> + * 64-bit userspace, they are already compatible. >> + */ >> + return pty_bsd_ioctl(tty, cmd, arg); >> +} >> + > > This looks correct but unnecessary, you can simply point both > function pointers to the same function: They have different types, since they have different return types: int (*ioctl)(struct tty_struct *tty, unsigned int cmd, unsigned long arg); long (*compat_ioctl)(struct tty_struct *tty, unsigned int cmd, unsigned long arg); If you like, I can change (*ioctl) to return longs as well, and then change all of the call-sites (since unlocked_ioctl also returns long).
On Tue, Jun 6, 2017 at 1:05 PM, Aleksa Sarai <asarai@suse.de> wrote: >>> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c >>> index 65799575c666..2a6bd9ae3f8b 100644 >>> --- a/drivers/tty/pty.c >>> +++ b/drivers/tty/pty.c >>> @@ -481,6 +481,16 @@ static int pty_bsd_ioctl(struct tty_struct *tty, >>> return -ENOIOCTLCMD; >>> } >>> >>> +static long pty_bsd_compat_ioctl(struct tty_struct *tty, >>> + unsigned int cmd, unsigned long arg) >>> +{ >>> + /* >>> + * PTY ioctls don't require any special translation between >>> 32-bit and >>> + * 64-bit userspace, they are already compatible. >>> + */ >>> + return pty_bsd_ioctl(tty, cmd, arg); >>> +} >>> + >> >> >> This looks correct but unnecessary, you can simply point both >> function pointers to the same function: > > > They have different types, since they have different return types: > > int (*ioctl)(struct tty_struct *tty, > unsigned int cmd, unsigned long arg); > long (*compat_ioctl)(struct tty_struct *tty, > unsigned int cmd, unsigned long arg); > > If you like, I can change (*ioctl) to return longs as well, and then change > all of the call-sites (since unlocked_ioctl also returns long). Ah, my mistake. In most other data structures that have a compat_ioctl callback pointer, the prototypes are the same, and I had not realized that tty_operations is an exception. Arnd
diff --git a/Makefile b/Makefile index 470bd4d9513a..fb689286d83a 100644 --- a/Makefile +++ b/Makefile @@ -401,6 +401,7 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -fno-common \ -Werror-implicit-function-declaration \ -Wno-format-security \ + -Wno-error=int-in-bool-context \ -std=gnu89 $(call cc-option,-fno-PIE) diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 65799575c666..2a6bd9ae3f8b 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -481,6 +481,16 @@ static int pty_bsd_ioctl(struct tty_struct *tty, return -ENOIOCTLCMD; } +static long pty_bsd_compat_ioctl(struct tty_struct *tty, + unsigned int cmd, unsigned long arg) +{ + /* + * PTY ioctls don't require any special translation between 32-bit and + * 64-bit userspace, they are already compatible. + */ + return pty_bsd_ioctl(tty, cmd, arg); +} + static int legacy_count = CONFIG_LEGACY_PTY_COUNT; /* * not really modular, but the easiest way to keep compat with existing @@ -502,6 +512,7 @@ static const struct tty_operations master_pty_ops_bsd = { .chars_in_buffer = pty_chars_in_buffer, .unthrottle = pty_unthrottle, .ioctl = pty_bsd_ioctl, + .compat_ioctl = pty_bsd_compat_ioctl, .cleanup = pty_cleanup, .resize = pty_resize, .remove = pty_remove @@ -609,6 +620,16 @@ static int pty_unix98_ioctl(struct tty_struct *tty, return -ENOIOCTLCMD; } +static long pty_unix98_compat_ioctl(struct tty_struct *tty, + unsigned int cmd, unsigned long arg) +{ + /* + * PTY ioctls don't require any special translation between 32-bit and + * 64-bit userspace, they are already compatible. + */ + return pty_unix98_ioctl(tty, cmd, arg); +} + /** * ptm_unix98_lookup - find a pty master * @driver: ptm driver @@ -681,6 +702,7 @@ static const struct tty_operations ptm_unix98_ops = { .chars_in_buffer = pty_chars_in_buffer, .unthrottle = pty_unthrottle, .ioctl = pty_unix98_ioctl, + .compat_ioctl = pty_unix98_compat_ioctl, .resize = pty_resize, .cleanup = pty_cleanup }; diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c index 6116d5275a3e..112b3e1e20e3 100644 --- a/fs/compat_ioctl.c +++ b/fs/compat_ioctl.c @@ -866,8 +866,6 @@ COMPATIBLE_IOCTL(TIOCGDEV) COMPATIBLE_IOCTL(TIOCCBRK) COMPATIBLE_IOCTL(TIOCGSID) COMPATIBLE_IOCTL(TIOCGICOUNT) -COMPATIBLE_IOCTL(TIOCGPKT) -COMPATIBLE_IOCTL(TIOCGPTLCK) COMPATIBLE_IOCTL(TIOCGEXCL) /* Little t */ COMPATIBLE_IOCTL(TIOCGETD) @@ -883,16 +881,12 @@ COMPATIBLE_IOCTL(TIOCMGET) COMPATIBLE_IOCTL(TIOCMBIC) COMPATIBLE_IOCTL(TIOCMBIS) COMPATIBLE_IOCTL(TIOCMSET) -COMPATIBLE_IOCTL(TIOCPKT) COMPATIBLE_IOCTL(TIOCNOTTY) COMPATIBLE_IOCTL(TIOCSTI) COMPATIBLE_IOCTL(TIOCOUTQ) COMPATIBLE_IOCTL(TIOCSPGRP) COMPATIBLE_IOCTL(TIOCGPGRP) -COMPATIBLE_IOCTL(TIOCGPTN) -COMPATIBLE_IOCTL(TIOCSPTLCK) COMPATIBLE_IOCTL(TIOCSERGETLSR) -COMPATIBLE_IOCTL(TIOCSIG) #ifdef TIOCSRS485 COMPATIBLE_IOCTL(TIOCSRS485) #endif
In order to avoid future diversions between fs/compat_ioctl.c and drivers/tty/pty.c, define .compat_ioctl callbacks for the relevant tty_operations structs. Since both pty_unix98_ioctl() and pty_bsd_ioctl() are compatible between 32-bit and 64-bit userspace no special translation is required. Signed-off-by: Aleksa Sarai <asarai@suse.de> --- Makefile | 1 + drivers/tty/pty.c | 22 ++++++++++++++++++++++ fs/compat_ioctl.c | 6 ------ 3 files changed, 23 insertions(+), 6 deletions(-)