[PATCH:xf86-input-keyboard 18/21] Fix wskbd handling when VT switching.

[Thomas Klausner]

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 <macallan at NetBSD.org>
> > Signed-off-by: Thomas Klausner <wiz at NetBSD.org>
> > ---
> >  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
-------------- next part --------------
>From dfb06978c1f45416f591967690b478b6eeecdccb Mon Sep 17 00:00:00 2001
From: Thomas Klausner <wiz at NetBSD.org>
Date: Mon, 29 Jul 2013 09:11:18 +0200
Subject: [PATCH:xf86-input-keyboard 23/23] Factor out common code.

Suggested by Walter Harms <wharms at bfs.de>.

Signed-off-by: Thomas Klausner <wiz at NetBSD.org>
---
 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