Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question on keyboard event devices in linux #66

Open
mik1234mc opened this issue Mar 10, 2022 · 5 comments
Open

Question on keyboard event devices in linux #66

mik1234mc opened this issue Mar 10, 2022 · 5 comments

Comments

@mik1234mc
Copy link

mik1234mc commented Mar 10, 2022

Hi all,

I have a custom keypad served with uinput (or now with libevdev) in linux-arm host. This keypad is not hooked to any tty, but it creates just /dev/input/eventX device. I checked the repo and kbd_event.c is now deprecated. My question is what is the recommended solution to my setup? Should I bring back kbd_event.c into the compilation or there is another way the event device can be somehow redirected to tty and then standard kbd_tty.c driver could be used?

I dont use X11.

Thank you for you suggestions,
Michael

@ghaerr
Copy link
Owner

ghaerr commented Mar 10, 2022

Hello @mik1234mc,

Thank you for your interest in MIcrowindows. I would suggest using kbd_event.c and moving it out of the deprecated/ folder and see whether it works for you unmodified, or with small changes. Let me know if you need additional help.

Than you!

@mik1234mc
Copy link
Author

Thank you for fast reply, Greg. The initial bring-up was quite seamless. I just needed to delete some init code in the kbd_event.c. In the end, I migrated to specify the device via env variable. See my patch below if interested.

diff --git a/src/drivers/Objects.rules b/src/drivers/Objects.rules
index c5d0961..613ed0d 100644
--- a/src/drivers/Objects.rules
+++ b/src/drivers/Objects.rules
@@ -185,6 +185,10 @@ ifeq ($(KEYBOARD), SCANKBD)
 MW_CORE_OBJS += $(MW_DIR_OBJ)/drivers/kbd_ttyscan.o
 endif
 
+ifeq ($(KEYBOARD), EVENTKBD)
+MW_CORE_OBJS += $(MW_DIR_OBJ)/drivers/deprecated/kbd_event.o
+endif
+
 #
 # Other
 #
diff --git a/src/drivers/deprecated/kbd_event.c b/src/drivers/deprecated/kbd_event.c
index b52aaee..45fd583 100644
--- a/src/drivers/deprecated/kbd_event.c
+++ b/src/drivers/deprecated/kbd_event.c
@@ -23,34 +23,24 @@ static MWKEYMOD curmodif = 0, allmodif = 0;
 static int KBD_Open(KBDDEVICE *pkd)
 {
        int i, r;
-       char fname[64];
-       for (i = 0; i < 32; i++)
+       char* kbd;
+
+    // "/dev/input/eventX" string is expected
+    kbd = getenv("KBDEVENTDEVICE");
+
+       fd = open(kbd, O_RDONLY | O_NONBLOCK);
+       if (fd < 0) 
        {
-               sprintf(fname, "/sys/class/input/event%d/device/capabilities/ev", i);
-               fd = open(fname, O_RDONLY);
-               if (fd < 0)
-                       continue;
-               r = read(fd, fname, sizeof(fname));
-               close(fd);
-               if (r <= 0)
-                       continue;
-    fname[r - 1] = '\0';
-               if ((strtoul(fname, NULL, 16) & (1 << EV_MSC)) == 0)
-                       continue;
-    sprintf(fname, "/dev/input/event%d", i);
-               fd = open(fname, O_RDONLY | O_NONBLOCK);
-               if (fd < 0)
-                       continue;
-               curmodif = 0;
-               /* TODO: Assign allmodif */
-               allmodif = MWKMOD_LSHIFT | MWKMOD_RSHIFT| MWKMOD_LCTRL |
-                        MWKMOD_RCTRL | MWKMOD_LALT | MWKMOD_RALT |
-                       MWKMOD_LMETA | MWKMOD_RMETA | MWKMOD_NUM |
-                        MWKMOD_CAPS | MWKMOD_ALTGR | MWKMOD_SCR;
-    return fd;
+        EPRINTF("Error %d opening keyboard input device\n", errno);
+           return errno;
        }
-       EPRINTF("Error %d opening keyboard input device\n", errno);
-       return errno;
+       curmodif = 0;
+       /* TODO: Assign allmodif */
+       allmodif = MWKMOD_LSHIFT | MWKMOD_RSHIFT| MWKMOD_LCTRL |
+                                       MWKMOD_RCTRL | MWKMOD_LALT | MWKMOD_RALT |
+               MWKMOD_LMETA | MWKMOD_RMETA | MWKMOD_NUM |
+                                       MWKMOD_CAPS | MWKMOD_ALTGR | MWKMOD_SCR;
+    return fd;
 }
 
 /*

@lpsantil
Copy link

+    // "/dev/input/eventX" string is expected
+    kbd = getenv("KBDEVENTDEVICE");
+
+       fd = open(kbd, O_RDONLY | O_NONBLOCK);

This seems unsafe. You might want to check your input before attempting to open it.

@mik1234mc
Copy link
Author

The open() call is followed by:

        if (fd < 0) 
	{
            EPRINTF("Error %d opening keyboard input device\n", errno);
	    return errno;
	}

Isn't this sufficient check?

@ghaerr
Copy link
Owner

ghaerr commented Mar 15, 2022

This seems unsafe. You might want to check your input before attempting to open it.

Isn't this sufficient check?

I think @lpsantil was referring to the potential security issue of opening a pathname sourced from an environment variable.
I don't have a comment on that, other than the security issue would highly depend on where this code is used.

On another matter I just noticed, the folllowing code returns 'errno' on an error; it should return -1:

if (fd < 0) 
	{
            EPRINTF("Error %d opening keyboard input device\n", errno);
	    return errno;
	}

The 'errno' return, always being positive, will be confused by Microwindows as having returned a valid keyboard file descriptor with the value of the positive errno value. All places in this keyboard driver that return errno should return -1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants