[RFC/PATCH v1 1/3] fix decoder of ioctl EVIOCGBIT

[Dmitry V. Levin]

On Fri, Aug 10, 2018 at 07:12:42PM +0800, Zhibin Li wrote:
> EV_SYN in EVIOCGBIT should be decoded to get other event types EV_*, not
> the precise event codes SYN_*.
> 
> * evdev.c: Include "xlat/evdev_ev.h" and remove "xlat/evdev_sync.h".
> (decode_bitset_): Fix the incorrect comparison between tcp->u_rval and
> max_nr.
> (bit_ioctl) <case EV_SYN>: Use evdev_ev and XT_NORMAL in decode_bitset
> invocation instead.
> * ioctl.c: Print 0 instead of EV_SYN if nr equals to 0x20.
> ---
>  evdev.c | 10 +++++-----
>  ioctl.c |  6 +++++-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/evdev.c b/evdev.c
> index 7ca15c9d..34732a2d 100644
> --- a/evdev.c
> +++ b/evdev.c
> @@ -37,6 +37,7 @@
>  # include <linux/input.h>
>  
>  # include "xlat/evdev_autorepeat.h"
> +# include "xlat/evdev_ev.h"

This way "xlat/evdev_ev.h" would be included by two different source
files, which would end up with static evdev_ev[] being defined twice.

This case looks similar to evdev_abs.  Let's avoid this duplication
by adding a declaration to defs.h and making evdev.c the place where
evdev_ev is defined.

For example,

Subject: [PATCH] evdev: fix decoding of EVIOCGBIT(0, ...)

There is a comment in drivers/input/evdev.c which says:
/* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */

That is, EVIOCGBIT(0, ...) should return a bit mask with supported
event types instead of SYN_* event codes.

* defs.h (evdev_ev): New prototype.
* evdev.c: Include "xlat/evdev_ev.h" and remove "xlat/evdev_sync.h".
(bit_ioctl) <case EV_SYN>: Replace EV_SYN with 0, use evdev_ev
with XT_SORTED in decode_bitset invocation instead.
* ioctl.c: Do not include "xlat/evdev_ev.h".
(evdev_decode_number): Print nr == 0x20 as "0" instead of "EV_SYN".
* tests/ioctl_evdev.c (main): Use 0 instead of EV_SYN in EVIOCGBIT
output.
* xlat/evdev_sync.in: Remove.

diff --git a/defs.h b/defs.h
index 0d4bf82b3..2c19dd3a4 100644
--- a/defs.h
+++ b/defs.h
@@ -330,6 +330,7 @@ extern const struct xlat evdev_abs[];
 /** Number of elements in evdev_abs array without the terminating record. */
 extern const size_t evdev_abs_size;
 
+extern const struct xlat evdev_ev[];
 extern const struct xlat iffflags[];
 extern const struct xlat ip_type_of_services[];
 extern const struct xlat ipc_private[];
diff --git a/evdev.c b/evdev.c
index 3c1aaa8e2..cae2ef154 100644
--- a/evdev.c
+++ b/evdev.c
@@ -30,6 +30,7 @@
 #include "defs.h"
 
 #include "xlat/evdev_abs.h"
+#include "xlat/evdev_ev.h"
 
 #ifdef HAVE_LINUX_INPUT_H
 
@@ -47,7 +48,6 @@
 # include "xlat/evdev_relative_axes.h"
 # include "xlat/evdev_snd.h"
 # include "xlat/evdev_switch.h"
-# include "xlat/evdev_sync.h"
 
 # ifndef SYN_MAX
 #  define SYN_MAX 0xf
@@ -258,9 +258,9 @@ bit_ioctl(struct tcb *const tcp, const unsigned int ev_nr,
 	  const kernel_ulong_t arg)
 {
 	switch (ev_nr) {
-		case EV_SYN:
-			return decode_bitset(tcp, arg, evdev_sync,
-					     SYN_MAX, "SYN_???", XT_INDEXED);
+		case 0:
+			return decode_bitset(tcp, arg, evdev_ev,
+					     EV_MAX, "EV_???", XT_SORTED);
 		case EV_KEY:
 			return decode_bitset(tcp, arg, evdev_keycode,
 					     KEY_MAX, "KEY_???", XT_INDEXED);
diff --git a/ioctl.c b/ioctl.c
index 93fb52631..66b10ec4c 100644
--- a/ioctl.c
+++ b/ioctl.c
@@ -33,8 +33,6 @@
 #include <linux/ioctl.h>
 #include "xlat/ioctl_dirs.h"
 
-#include "xlat/evdev_ev.h"
-
 static int
 compare(const void *a, const void *b)
 {
@@ -99,7 +97,10 @@ evdev_decode_number(const unsigned int code)
 
 	if (nr >= 0x20 && nr <= 0x20 + 0x1f) {
 		tprints("EVIOCGBIT(");
-		printxval(evdev_ev, nr - 0x20, "EV_???");
+		if (nr == 0x20)
+			tprintf("0");
+		else
+			printxval(evdev_ev, nr - 0x20, "EV_???");
 		tprintf(", %u)", _IOC_SIZE(code));
 		return 1;
 	} else if (nr >= 0x40 && nr <= 0x40 + 0x3f) {
diff --git a/tests/ioctl_evdev.c b/tests/ioctl_evdev.c
index 5eacac024..071b6b0dd 100644
--- a/tests/ioctl_evdev.c
+++ b/tests/ioctl_evdev.c
@@ -138,7 +138,7 @@ main(void)
 	TEST_NULL_ARG_EX(EVIOCGABS(0x3f), "EVIOCGABS(0x3f /* ABS_??? */)");
 	TEST_NULL_ARG_EX(EVIOCSABS(0x3f), "EVIOCSABS(0x3f /* ABS_??? */)");
 
-	TEST_NULL_ARG(EVIOCGBIT(EV_SYN, 0));
+	TEST_NULL_ARG(EVIOCGBIT(0, 0));
 	TEST_NULL_ARG(EVIOCGBIT(EV_KEY, 1));
 	TEST_NULL_ARG(EVIOCGBIT(EV_REL, 2));
 	TEST_NULL_ARG(EVIOCGBIT(EV_ABS, 3));
diff --git a/xlat/evdev_sync.in b/xlat/evdev_sync.in
deleted file mode 100644
index ca9ea5035..000000000
--- a/xlat/evdev_sync.in
+++ /dev/null
@@ -1,5 +0,0 @@
-#value_indexed
-SYN_REPORT	0
-SYN_CONFIG	1
-SYN_MT_REPORT	2
-SYN_DROPPED	3

What do you think?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180819/c3192e67/attachment.bin>