On Sat, Jul 27, 2013 at 11:55:27AM +0200, walter harms wrote:
>
>
> Am 26.07.2013 23:24, schrieb Thomas Klausner:
> > When using /dev/wskbd* we need to close the device when VT switching
> > out of X, and open it again when switching back.
> >
> > From Michael Lorenz <[email protected]>
> > Signed-off-by: Thomas Klausner <[email protected]>
> > ---
> > src/bsd_kbd.c | 35 ++++++++++++++++++++++++++++++++++-
> > src/xf86OSKbd.h | 1 +
> > 2 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/bsd_kbd.c b/src/bsd_kbd.c
> > index 70a3ff1..0615cf2 100644
> > --- a/src/bsd_kbd.c
> > +++ b/src/bsd_kbd.c
> > @@ -204,6 +204,24 @@ KbdOn(InputInfoPtr pInfo, int what)
> > break;
> > #endif
> > }
> > + } else {
> > + switch (pKbd->consType) {
> > +#ifdef WSCONS_SUPPORT
> > + case WSCONS:
> > + if ((pKbd->wsKbdDev[0] != 0) && (pInfo->fd == -1)) {
> > + xf86Msg(X_INFO, "opening %s\n", pKbd->wsKbdDev);
> > + pInfo->fd = open(pKbd->wsKbdDev, O_RDONLY | O_NONBLOCK
> > | O_EXCL);
> > +#ifdef WSKBDIO_SETVERSION
> > + int version = WSKBDIO_EVENT_VERSION;
> > + if (ioctl(pInfo->fd, WSKBDIO_SETVERSION, &version) ==
> > -1) {
> > + xf86Msg(X_WARNING, "%s: cannot set version\n",
> > pInfo->name);
> > + return FALSE;
> > + }
> > +#endif
>
> I remember this from a patch before.
> IMHO you can impove readablity be using a function to set version.
> like:
>
> int set_version(pinfo,int version) {
> if (ioctl(pInfo->fd, WSKBDIO_SETVERSION, &version) == -1) {
> xf86Msg(X_WARNING, "%s: cannot set version\n", pInfo->name);
> return -1; /* do we need to know ??? */
> }
> return 0;
> }
>
>
> #ifdef WSKBDIO_SETVERSION
> if ( set_version(pinfo, WSKBDIO_EVENT_VERSION ) <0 ) return FALSE;
> #endif
>
> when WSKBDIO_EVENT_VERSION is defined unconditionaly you let set_version()
> return 0,
> and has reduced the #ifdef level by 1.
Ok, I've done that. Patch attached.
> I would also think again about the open(). In case open fails
> the user get an error like "cannot set version", sending him on the wrong
> path.
> adding an xopen() like:
>
> int xopen(char *name, int mode) {
> int fd=open (name,mode);
> if ( fd < 0 ) error("failed to open %s\n",name);
> return fd;
> }
> In this case you could even ignore the error from ioctl() because it is clear
> what the acutal
> problem is.
I don't think this improves the code in this particular piece of code.
Thanks for the comments,
Thomas
>From dfb06978c1f45416f591967690b478b6eeecdccb Mon Sep 17 00:00:00 2001
From: Thomas Klausner <[email protected]>
Date: Mon, 29 Jul 2013 09:11:18 +0200
Subject: [PATCH:xf86-input-keyboard 23/23] Factor out common code.
Suggested by Walter Harms <[email protected]>.
Signed-off-by: Thomas Klausner <[email protected]>
---
src/bsd_kbd.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/src/bsd_kbd.c b/src/bsd_kbd.c
index 1e0531e..175c544 100644
--- a/src/bsd_kbd.c
+++ b/src/bsd_kbd.c
@@ -39,6 +39,21 @@ typedef struct {
struct termios kbdtty;
} BsdKbdPrivRec, *BsdKbdPrivPtr;
+#ifdef WSCONS_SUPPORT
+static Bool
+WSSetVersion(int fd, const char *name)
+{
+#ifdef WSKBDIO_SETVERSION
+ int version = WSKBDIO_EVENT_VERSION;
+ if (ioctl(fd, WSKBDIO_SETVERSION, &version) == -1) {
+ xf86Msg(X_WARNING, "%s: cannot set version\n", name);
+ return FALSE;
+ }
+#endif
+ return TRUE;
+}
+#endif
+
static
int KbdInit(InputInfoPtr pInfo, int what)
{
@@ -211,13 +226,8 @@ KbdOn(InputInfoPtr pInfo, int what)
if ((pKbd->wsKbdDev[0] != 0) && (pInfo->fd == -1)) {
xf86Msg(X_INFO, "opening %s\n", pKbd->wsKbdDev);
pInfo->fd = open(pKbd->wsKbdDev, O_RDONLY | O_NONBLOCK
| O_EXCL);
-#ifdef WSKBDIO_SETVERSION
- int version = WSKBDIO_EVENT_VERSION;
- if (ioctl(pInfo->fd, WSKBDIO_SETVERSION, &version) ==
-1) {
- xf86Msg(X_WARNING, "%s: cannot set version\n",
pInfo->name);
+ if (WSSetVersion(pInfo->fd, pInfo->name) == FALSE)
return FALSE;
- }
-#endif
}
break;
#endif
@@ -416,13 +426,8 @@ OpenKeyboard(InputInfoPtr pInfo)
#ifdef WSCONS_SUPPORT
if( prot == PROT_WSCONS) {
pKbd->consType = WSCONS;
-#ifdef WSKBDIO_SETVERSION
- int version = WSKBDIO_EVENT_VERSION;
- if (ioctl(pInfo->fd, WSKBDIO_SETVERSION, &version) == -1) {
- xf86Msg(X_WARNING, "%s: cannot set version\n", pInfo->name);
- return FALSE;
- }
-#endif
+ if (WSSetVersion(pInfo->fd, pInfo->name) == FALSE)
+ return FALSE;
/* Find out keyboard type */
if (ioctl(pInfo->fd, WSKBDIO_GTYPE, &(pKbd->wsKbdType)) == -1) {
xf86Msg(X_ERROR, "%s: cannot get keyboard type", pInfo->name);
--
1.8.3.3
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
