[Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Anthony Liguori]

Revision: 6721
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6721
Author:   aliguori
Date:     2009-03-06 20:27:10 +0000 (Fri, 06 Mar 2009)
Log Message:
-----------
Refactor keymap code to avoid duplication ("Daniel P. Berrange")

Each of the graphical frontends #include a .c file, for keymap code
resulting in duplicated definitions & duplicated compiled code. A
couple of small changes allowed this to be sanitized, so instead of
doing a #include "keymaps.c", duplicating all code, we can have a
shared keymaps.h file, and only compile code once. This allows the
next patch to move the VncState struct out into a header file without
causing clashing definitions.


 Makefile      |    9 +++++---
 b/keymaps.h   |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 curses.c      |    3 --
 curses_keys.h |    9 +++-----
 keymaps.c     |   45 ++++++++++++++++---------------------------
 sdl.c         |    3 --
 sdl_keysym.h  |    7 ++----
 vnc.c         |    5 +---
 vnc_keysym.h  |    7 ++----
 9 files changed, 97 insertions(+), 51 deletions(-)

   Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Modified Paths:
--------------
    trunk/Makefile
    trunk/curses.c
    trunk/curses_keys.h
    trunk/keymaps.c
    trunk/sdl.c
    trunk/sdl_keysym.h
    trunk/vnc.c
    trunk/vnc_keysym.h

Modified: trunk/Makefile
===================================================================
--- trunk/Makefile	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/Makefile	2009-03-06 20:27:10 UTC (rev 6721)
@@ -137,6 +137,7 @@
 AUDIO_OBJS+= wavcapture.o
 OBJS+=$(addprefix audio/, $(AUDIO_OBJS))
 
+OBJS+=keymaps.o
 ifdef CONFIG_SDL
 OBJS+=sdl.o x_keymap.o
 endif
@@ -161,15 +162,17 @@
 
 cocoa.o: cocoa.m
 
-sdl.o: sdl.c keymaps.c sdl_keysym.h
+keymaps.o: keymaps.c keymaps.h
 
+sdl.o: sdl.c keymaps.h sdl_keysym.h
+
 sdl.o audio/sdlaudio.o: CFLAGS += $(SDL_CFLAGS)
 
-vnc.o: vnc.c keymaps.c sdl_keysym.h vnchextile.h d3des.c d3des.h
+vnc.o: vnc.c keymaps.h sdl_keysym.h vnchextile.h d3des.c d3des.h
 
 vnc.o: CFLAGS += $(CONFIG_VNC_TLS_CFLAGS)
 
-curses.o: curses.c keymaps.c curses_keys.h
+curses.o: curses.c keymaps.h curses_keys.h
 
 bt-host.o: CFLAGS += $(CONFIG_BLUEZ_CFLAGS)
 

Modified: trunk/curses.c
===================================================================
--- trunk/curses.c	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/curses.c	2009-03-06 20:27:10 UTC (rev 6721)
@@ -158,7 +158,6 @@
 /* generic keyboard conversion */
 
 #include "curses_keys.h"
-#include "keymaps.c"
 
 static kbd_layout_t *kbd_layout = 0;
 static int keycode2keysym[CURSES_KEYS];
@@ -311,7 +310,7 @@
         keyboard_layout = "en-us";
 #endif
     if(keyboard_layout) {
-        kbd_layout = init_keyboard_layout(keyboard_layout);
+        kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
         if (!kbd_layout)
             exit(1);
     }

Modified: trunk/curses_keys.h
===================================================================
--- trunk/curses_keys.h	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/curses_keys.h	2009-03-06 20:27:10 UTC (rev 6721)
@@ -21,6 +21,10 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
+#include "keymaps.h"
+
+
 #define KEY_RELEASE         0x80
 #define KEY_MASK            0x7f
 #define SHIFT_CODE          0x2a
@@ -239,11 +243,6 @@
 
 };
 
-typedef struct {
-	const char* name;
-	int keysym;
-} name2keysym_t;
-
 static const name2keysym_t name2keysym[] = {
     /* Plain ASCII */
     { "space", 0x020 },

Modified: trunk/keymaps.c
===================================================================
--- trunk/keymaps.c	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/keymaps.c	2009-03-06 20:27:10 UTC (rev 6721)
@@ -22,35 +22,21 @@
  * THE SOFTWARE.
  */
 
-static int get_keysym(const char *name)
+#include "keymaps.h"
+#include "sysemu.h"
+
+static int get_keysym(const name2keysym_t *table,
+		      const char *name)
 {
     const name2keysym_t *p;
-    for(p = name2keysym; p->name != NULL; p++) {
+    for(p = table; p->name != NULL; p++) {
         if (!strcmp(p->name, name))
             return p->keysym;
     }
     return 0;
 }
 
-struct key_range {
-    int start;
-    int end;
-    struct key_range *next;
-};
 
-#define MAX_NORMAL_KEYCODE 512
-#define MAX_EXTRA_COUNT 256
-typedef struct {
-    uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
-    struct {
-	int keysym;
-	uint16_t keycode;
-    } keysym2keycode_extra[MAX_EXTRA_COUNT];
-    int extra_count;
-    struct key_range *keypad_range;
-    struct key_range *numlock_range;
-} kbd_layout_t;
-
 static void add_to_key_range(struct key_range **krp, int code) {
     struct key_range *kr;
     for (kr = *krp; kr; kr = kr->next) {
@@ -73,7 +59,8 @@
     }
 }
 
-static kbd_layout_t *parse_keyboard_layout(const char *language,
+static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
+					   const char *language,
 					   kbd_layout_t * k)
 {
     FILE *f;
@@ -102,7 +89,7 @@
 	if (!strncmp(line, "map ", 4))
 	    continue;
 	if (!strncmp(line, "include ", 8)) {
-	    parse_keyboard_layout(line + 8, k);
+	    parse_keyboard_layout(table, line + 8, k);
         } else {
 	    char *end_of_keysym = line;
 	    while (*end_of_keysym != 0 && *end_of_keysym != ' ')
@@ -110,7 +97,7 @@
 	    if (*end_of_keysym) {
 		int keysym;
 		*end_of_keysym = 0;
-		keysym = get_keysym(line);
+		keysym = get_keysym(table, line);
 		if (keysym == 0) {
                     //		    fprintf(stderr, "Warning: unknown keysym %s\n", line);
 		} else {
@@ -154,12 +141,14 @@
     return k;
 }
 
-static void *init_keyboard_layout(const char *language)
+
+void *init_keyboard_layout(const name2keysym_t *table, const char *language)
 {
-    return parse_keyboard_layout(language, 0);
+    return parse_keyboard_layout(table, language, 0);
 }
 
-static int keysym2scancode(void *kbd_layout, int keysym)
+
+int keysym2scancode(void *kbd_layout, int keysym)
 {
     kbd_layout_t *k = kbd_layout;
     if (keysym < MAX_NORMAL_KEYCODE) {
@@ -180,7 +169,7 @@
     return 0;
 }
 
-static inline int keycode_is_keypad(void *kbd_layout, int keycode)
+int keycode_is_keypad(void *kbd_layout, int keycode)
 {
     kbd_layout_t *k = kbd_layout;
     struct key_range *kr;
@@ -191,7 +180,7 @@
     return 0;
 }
 
-static inline int keysym_is_numlock(void *kbd_layout, int keysym)
+int keysym_is_numlock(void *kbd_layout, int keysym)
 {
     kbd_layout_t *k = kbd_layout;
     struct key_range *kr;

Modified: trunk/sdl.c
===================================================================
--- trunk/sdl.c	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/sdl.c	2009-03-06 20:27:10 UTC (rev 6721)
@@ -109,7 +109,6 @@
 /* generic keyboard conversion */
 
 #include "sdl_keysym.h"
-#include "keymaps.c"
 
 static kbd_layout_t *kbd_layout = NULL;
 
@@ -677,7 +676,7 @@
         keyboard_layout = "en-us";
 #endif
     if(keyboard_layout) {
-        kbd_layout = init_keyboard_layout(keyboard_layout);
+        kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
         if (!kbd_layout)
             exit(1);
     }

Modified: trunk/sdl_keysym.h
===================================================================
--- trunk/sdl_keysym.h	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/sdl_keysym.h	2009-03-06 20:27:10 UTC (rev 6721)
@@ -1,7 +1,6 @@
-typedef struct {
-	const char* name;
-	int keysym;
-} name2keysym_t;
+
+#include "keymaps.h"
+
 static const name2keysym_t name2keysym[]={
 /* ascii */
     { "space",                0x020},

Modified: trunk/vnc.c
===================================================================
--- trunk/vnc.c	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/vnc.c	2009-03-06 20:27:10 UTC (rev 6721)
@@ -36,7 +36,6 @@
 
 #include "vnc.h"
 #include "vnc_keysym.h"
-#include "keymaps.c"
 #include "d3des.h"
 
 #ifdef CONFIG_VNC_TLS
@@ -2422,9 +2421,9 @@
     vs->ds = ds;
 
     if (keyboard_layout)
-        vs->kbd_layout = init_keyboard_layout(keyboard_layout);
+        vs->kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
     else
-        vs->kbd_layout = init_keyboard_layout("en-us");
+        vs->kbd_layout = init_keyboard_layout(name2keysym, "en-us");
 
     if (!vs->kbd_layout)
 	exit(1);

Modified: trunk/vnc_keysym.h
===================================================================
--- trunk/vnc_keysym.h	2009-03-06 20:27:05 UTC (rev 6720)
+++ trunk/vnc_keysym.h	2009-03-06 20:27:10 UTC (rev 6721)
@@ -1,7 +1,6 @@
-typedef struct {
-	const char* name;
-	int keysym;
-} name2keysym_t;
+
+#include "keymaps.h"
+
 static const name2keysym_t name2keysym[]={
 /* ascii */
     { "space",                0x020},

Re: [Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Stefan Weil]

Anthony Liguori schrieb:
> Revision: 6721
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6721
> Author:   aliguori
> Date:     2009-03-06 20:27:10 +0000 (Fri, 06 Mar 2009)
> Log Message:
> -----------
> Refactor keymap code to avoid duplication ("Daniel P. Berrange")
>
> Each of the graphical frontends #include a .c file, for keymap code
> resulting in duplicated definitions & duplicated compiled code. A
> couple of small changes allowed this to be sanitized, so instead of
> doing a #include "keymaps.c", duplicating all code, we can have a
> shared keymaps.h file, and only compile code once. This allows the
> next patch to move the VncState struct out into a header file without
> causing clashing definitions.
>
>   


The new file keymaps.h is missing.

Re: [Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Johannes Schindelin]

Hi,

On Fri, 6 Mar 2009, Stefan Weil wrote:

> Anthony Liguori schrieb:
> > Revision: 6721
> >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6721
> > Author:   aliguori
> > Date:     2009-03-06 20:27:10 +0000 (Fri, 06 Mar 2009)
> > Log Message:
> > -----------
> > Refactor keymap code to avoid duplication ("Daniel P. Berrange")
> >
> > Each of the graphical frontends #include a .c file, for keymap code
> > resulting in duplicated definitions & duplicated compiled code. A
> > couple of small changes allowed this to be sanitized, so instead of
> > doing a #include "keymaps.c", duplicating all code, we can have a
> > shared keymaps.h file, and only compile code once. This allows the
> > next patch to move the VncState struct out into a header file without
> > causing clashing definitions.
> >
> >   
> 
> 
> The new file keymaps.h is missing.

Anthony, as you seem to use Git from time to time, you might be interested 
in using git-svn, to overcome such omissions.

Hth,
Dscho

Re: [Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Anthony Liguori]

Johannes Schindelin wrote:
>> The new file keymaps.h is missing.
>>     
>
> Anthony, as you seem to use Git from time to time, you might be interested 
> in using git-svn, to overcome such omissions.
>   

guilt failed me here.

I have scripts that pull patches from the mailing list (basically, 
git-am but a lot more forgiving) and converts that to a series of 
patches for guilt.  I think use the guilt tree on top of a git tree 
which I use to automatically build/test each patch in a series.  After 
this, I think do more exhaustive testing of the whole series applied.

I think have another script that will apply the whole series to SVN.  
It's smart enough to read the patch files to determine what files have 
been added/removed and issue the appropriate svn add/rm command.  
Basically, this is an svn-import command.

With this particular series, it didn't apply to trunk anymore so I did 
some fixups.  Unfortunately, guilt has a long standing bug where it can 
sometimes lose track of files introduced by a patch.  This happened so I 
had to manually git add && guilt refresh to add the files back to the 
patch.  The curious thing is that somehow, in the process of this, 
instead of having diff /dev/null b/foo.h, it did diff a/foo.h b/foo.h 
and treated foo.h as an empty file.  This means that my svn-import 
script didn't catch it as an added file.  I've fixed this by adding an 
svn status check to ensure no stray files are around.

What I'd really like to do, is switch over to just using git-am and then 
push the patches to a git tree.  But I jump through all of the above 
hoops to accommodate people that send patches that aren't git-am 
friendly (which is the majority today) and to morph command messages 
into the format that QEMU has traditionally used.

One of the issues of converting over to git is that Savannah doesn't 
really have a robust git hosting mechanism.  Each project can have one 
tree.  I also am not sure that there's a reasonable mechanism to do 
commit hooks to enable mailing list messages.  Savannah also requires 
that the main code repository be hosted in Savannah so it wouldn't be an 
option to continue using Savannah services but host the git tree 
somewhere else.

Regards,

Anthony Liguori

> Hth,
> Dscho
>
>   

Re: [Qemu-devel] [6721] Refactor keymap code to avoid duplication ("Daniel P.

[Johannes Schindelin]

Hi,

On Fri, 6 Mar 2009, Anthony Liguori wrote:

> Johannes Schindelin wrote:
> > > The new file keymaps.h is missing.
> > >     
> >
> > Anthony, as you seem to use Git from time to time, you might be 
> > interested in using git-svn, to overcome such omissions.
> 
> guilt failed me here.
> 
> I have scripts that pull patches from the mailing list (basically, 
> git-am but a lot more forgiving) and converts that to a series of 
> patches for guilt.  I think use the guilt tree on top of a git tree 
> which I use to automatically build/test each patch in a series.  After 
> this, I think do more exhaustive testing of the whole series applied.
> 
> I think have another script that will apply the whole series to SVN.  
> It's smart enough to read the patch files to determine what files have 
> been added/removed and issue the appropriate svn add/rm command.  
> Basically, this is an svn-import command.
> 
> With this particular series, it didn't apply to trunk anymore so I did 
> some fixups.  Unfortunately, guilt has a long standing bug where it can 
> sometimes lose track of files introduced by a patch.  This happened so I 
> had to manually git add && guilt refresh to add the files back to the 
> patch.  The curious thing is that somehow, in the process of this, 
> instead of having diff /dev/null b/foo.h, it did diff a/foo.h b/foo.h 
> and treated foo.h as an empty file.  This means that my svn-import 
> script didn't catch it as an added file. I've fixed this by adding an 
> svn status check to ensure no stray files are around.
> 
> What I'd really like to do, is switch over to just using git-am and then 
> push the patches to a git tree.  But I jump through all of the above 
> hoops to accommodate people that send patches that aren't git-am 
> friendly (which is the majority today) and to morph command messages 
> into the format that QEMU has traditionally used.

What exactly is failing?  Are the patches not even applicable if you use 
"patch < .git/rebase-apply/patch" after a failed patch?

> One of the issues of converting over to git is that Savannah doesn't 
> really have a robust git hosting mechanism.  Each project can have one 
> tree.  I also am not sure that there's a reasonable mechanism to do 
> commit hooks to enable mailing list messages.  Savannah also requires 
> that the main code repository be hosted in Savannah so it wouldn't be an 
> option to continue using Savannah services but host the git tree 
> somewhere else.

I do not see the problem.  Why not just push to Savannah's Git repository?  
Of course, Git being distributed, you can push to an arbitrary number of 
public repositories whenever you feel like it.

BTW thanks for the creation of the stable branch.  Maybe I find time later 
this week to find out if WinXP64 guests are still broken or not.

Ciao,
Dscho