<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sun, Aug 19, 2018 at 4:38 PM Dmitry V. Levin <<a href="mailto:ldv@altlinux.org">ldv@altlinux.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Aug 10, 2018 at 07:12:42PM +0800, Zhibin Li wrote:<br>
> EV_SYN in EVIOCGBIT should be decoded to get other event types EV_*, not<br>
> the precise event codes SYN_*.<br>
> <br>
> * evdev.c: Include "xlat/evdev_ev.h" and remove "xlat/evdev_sync.h".<br>
> (decode_bitset_): Fix the incorrect comparison between tcp->u_rval and<br>
> max_nr.<br>
> (bit_ioctl) <case EV_SYN>: Use evdev_ev and XT_NORMAL in decode_bitset<br>
> invocation instead.<br>
> * ioctl.c: Print 0 instead of EV_SYN if nr equals to 0x20.<br>
> ---<br>
> evdev.c | 10 +++++-----<br>
> ioctl.c | 6 +++++-<br>
> 2 files changed, 10 insertions(+), 6 deletions(-)<br>
> <br>
> diff --git a/evdev.c b/evdev.c<br>
> index 7ca15c9d..34732a2d 100644<br>
> --- a/evdev.c<br>
> +++ b/evdev.c<br>
> @@ -37,6 +37,7 @@<br>
> # include <linux/input.h><br>
> <br>
> # include "xlat/evdev_autorepeat.h"<br>
> +# include "xlat/evdev_ev.h"<br>
<br>
This way "xlat/evdev_ev.h" would be included by two different source<br>
files, which would end up with static evdev_ev[] being defined twice.<br></blockquote><div>I didn't notice that. Thanks for pointing it out :) </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This case looks similar to evdev_abs. Let's avoid this duplication<br>
by adding a declaration to defs.h and making evdev.c the place where<br>
evdev_ev is defined.<br>
<br>
For example,<br>
<br>
Subject: [PATCH] evdev: fix decoding of EVIOCGBIT(0, ...)<br>
<br>
There is a comment in drivers/input/evdev.c which says:<br>
/* EV_SYN==0 is EV_CNT, _not_ SYN_CNT, see EVIOCGBIT */<br>
<br>
That is, EVIOCGBIT(0, ...) should return a bit mask with supported<br>
event types instead of SYN_* event codes.<br>
<br>
* defs.h (evdev_ev): New prototype.<br>
* evdev.c: Include "xlat/evdev_ev.h" and remove "xlat/evdev_sync.h".<br>
(bit_ioctl) <case EV_SYN>: Replace EV_SYN with 0, use evdev_ev<br>
with XT_SORTED in decode_bitset invocation instead.<br>
* ioctl.c: Do not include "xlat/evdev_ev.h".<br>
(evdev_decode_number): Print nr == 0x20 as "0" instead of "EV_SYN".<br>
* tests/ioctl_evdev.c (main): Use 0 instead of EV_SYN in EVIOCGBIT<br>
output.<br>
* xlat/<a href="http://evdev_sync.in" rel="noreferrer" target="_blank">evdev_sync.in</a>: Remove.<br>
<br>
diff --git a/defs.h b/defs.h<br>
index 0d4bf82b3..2c19dd3a4 100644<br>
--- a/defs.h<br>
+++ b/defs.h<br>
@@ -330,6 +330,7 @@ extern const struct xlat evdev_abs[];<br>
/** Number of elements in evdev_abs array without the terminating record. */<br>
extern const size_t evdev_abs_size;<br>
<br>
+extern const struct xlat evdev_ev[];<br>
extern const struct xlat iffflags[];<br>
extern const struct xlat ip_type_of_services[];<br>
extern const struct xlat ipc_private[];<br>
diff --git a/evdev.c b/evdev.c<br>
index 3c1aaa8e2..cae2ef154 100644<br>
--- a/evdev.c<br>
+++ b/evdev.c<br>
@@ -30,6 +30,7 @@<br>
#include "defs.h"<br>
<br>
#include "xlat/evdev_abs.h"<br>
+#include "xlat/evdev_ev.h"<br>
<br>
#ifdef HAVE_LINUX_INPUT_H<br>
<br>
@@ -47,7 +48,6 @@<br>
# include "xlat/evdev_relative_axes.h"<br>
# include "xlat/evdev_snd.h"<br>
# include "xlat/evdev_switch.h"<br>
-# include "xlat/evdev_sync.h"<br>
<br>
# ifndef SYN_MAX<br>
# define SYN_MAX 0xf<br>
@@ -258,9 +258,9 @@ bit_ioctl(struct tcb *const tcp, const unsigned int ev_nr,<br>
const kernel_ulong_t arg)<br>
{<br>
switch (ev_nr) {<br>
- case EV_SYN:<br>
- return decode_bitset(tcp, arg, evdev_sync,<br>
- SYN_MAX, "SYN_???", XT_INDEXED);<br>
+ case 0:<br>
+ return decode_bitset(tcp, arg, evdev_ev,<br>
+ EV_MAX, "EV_???", XT_SORTED);<br>
case EV_KEY:<br>
return decode_bitset(tcp, arg, evdev_keycode,<br>
KEY_MAX, "KEY_???", XT_INDEXED);<br>
diff --git a/ioctl.c b/ioctl.c<br>
index 93fb52631..66b10ec4c 100644<br>
--- a/ioctl.c<br>
+++ b/ioctl.c<br>
@@ -33,8 +33,6 @@<br>
#include <linux/ioctl.h><br>
#include "xlat/ioctl_dirs.h"<br>
<br>
-#include "xlat/evdev_ev.h"<br>
-<br>
static int<br>
compare(const void *a, const void *b)<br>
{<br>
@@ -99,7 +97,10 @@ evdev_decode_number(const unsigned int code)<br>
<br>
if (nr >= 0x20 && nr <= 0x20 + 0x1f) {<br>
tprints("EVIOCGBIT(");<br>
- printxval(evdev_ev, nr - 0x20, "EV_???");<br>
+ if (nr == 0x20)<br>
+ tprintf("0");<br>
+ else<br>
+ printxval(evdev_ev, nr - 0x20, "EV_???");<br></blockquote><div><div>BTW, a few days ago eSyr said in the mail thread that this part doesn't go</div><div>well with the concept of xlat styles. Should I modified xlat/<a href="http://evdev_ev.in">evdev_ev.in</a></div><div>instead? But simply changing EV_SYN to 0 won't work because 0 isn't an</div><div>identifier which can be used as a macro name. Any possible solution that I</div><div>can check?</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
tprintf(", %u)", _IOC_SIZE(code));<br>
return 1;<br>
} else if (nr >= 0x40 && nr <= 0x40 + 0x3f) {<br>
diff --git a/tests/ioctl_evdev.c b/tests/ioctl_evdev.c<br>
index 5eacac024..071b6b0dd 100644<br>
--- a/tests/ioctl_evdev.c<br>
+++ b/tests/ioctl_evdev.c<br>
@@ -138,7 +138,7 @@ main(void)<br>
TEST_NULL_ARG_EX(EVIOCGABS(0x3f), "EVIOCGABS(0x3f /* ABS_??? */)");<br>
TEST_NULL_ARG_EX(EVIOCSABS(0x3f), "EVIOCSABS(0x3f /* ABS_??? */)");<br>
<br>
- TEST_NULL_ARG(EVIOCGBIT(EV_SYN, 0));<br>
+ TEST_NULL_ARG(EVIOCGBIT(0, 0));<br>
TEST_NULL_ARG(EVIOCGBIT(EV_KEY, 1));<br>
TEST_NULL_ARG(EVIOCGBIT(EV_REL, 2));<br>
TEST_NULL_ARG(EVIOCGBIT(EV_ABS, 3));<br>
diff --git a/xlat/<a href="http://evdev_sync.in" rel="noreferrer" target="_blank">evdev_sync.in</a> b/xlat/<a href="http://evdev_sync.in" rel="noreferrer" target="_blank">evdev_sync.in</a><br>
deleted file mode 100644<br>
index ca9ea5035..000000000<br>
--- a/xlat/<a href="http://evdev_sync.in" rel="noreferrer" target="_blank">evdev_sync.in</a><br>
+++ /dev/null<br>
@@ -1,5 +0,0 @@<br>
-#value_indexed<br>
-SYN_REPORT 0<br>
-SYN_CONFIG 1<br>
-SYN_MT_REPORT 2<br>
-SYN_DROPPED 3<br>
<br>
What do you think?<br>
<br>
<br>
-- <br>
ldv<br>
</blockquote></div></div>