[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 1/7] Rewrite keycode map to avoid a struct



From: "Daniel P. Berrange" <berrange redhat com>

Playing games with field offsets in a struct causes all sorts
of alignment warnings on ARM platforms

util/virkeycode.c: In function '__virKeycodeValueFromString':
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
     (*(typeof(field_type) *)((char *)(object) + field_offset))
       ^
util/virkeycode.c:91:28: note: in expansion of macro 'getfield'
         const char *name = getfield(virKeycodes + i, const char *, name_offset);
                            ^
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
     (*(typeof(field_type) *)((char *)(object) + field_offset))
       ^
util/virkeycode.c:94:20: note: in expansion of macro 'getfield'
             return getfield(virKeycodes + i, unsigned short, code_offset);
                    ^
util/virkeycode.c: In function '__virKeycodeValueTranslate':
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
     (*(typeof(field_type) *)((char *)(object) + field_offset))
       ^
util/virkeycode.c:127:13: note: in expansion of macro 'getfield'
         if (getfield(virKeycodes + i, unsigned short, from_offset) == key_value)
             ^
util/virkeycode.c:26:7: warning: cast increases required alignment of target type [-Wcast-align]
     (*(typeof(field_type) *)((char *)(object) + field_offset))
       ^
util/virkeycode.c:128:20: note: in expansion of macro 'getfield'
             return getfield(virKeycodes + i, unsigned short, to_offset);

There is no compelling reason to use a struct for the keycode
tables. It can easily just use an array of arrays instead,
avoiding all alignment problems

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---
 src/util/virkeycode-mapgen.py |  78 ++++++++++++++++++-------
 src/util/virkeycode.c         | 130 ++++++++++++++++++------------------------
 2 files changed, 110 insertions(+), 98 deletions(-)

diff --git a/src/util/virkeycode-mapgen.py b/src/util/virkeycode-mapgen.py
index d3d2aae..34de637 100755
--- a/src/util/virkeycode-mapgen.py
+++ b/src/util/virkeycode-mapgen.py
@@ -13,7 +13,22 @@ instead of keymaps.csv which is a mirror.
 import sys
 import re
 
-namecolums = (0,2,10)
+cols = (
+    ["linux", True],
+    ["linux", False],
+    ["os_x", True],
+    ["os_x", False],
+    ["atset1", False],
+    ["atset2", False],
+    ["atset3", False],
+    ["xt", False],
+    ["xt_kbd", False],
+    ["usb", False],
+    ["win32", True],
+    ["win32", False],
+    ["rfb", False],
+)
+
 xtkbdkey_index = 8
 
 def quotestring(str):
@@ -28,29 +43,48 @@ print '''
 # error do not use this; it is not a public header
 #endif
 
-struct keycode virKeycodes[] = {
 '''
 
 sys.stdin.readline() # eat the fist line.
 
+keycodes = []
+
+max = 0
+
 for line in sys.stdin.xreadlines():
-    a = re.match("([^,]*)," * 13 + "([^,]*)$", line[0:-1]).groups()
-    b = ""
-    rfbkey = 0
-    for i in namecolums:
-        b = b + (a[i] and quotestring(a[i]) or 'NULL') + ','
-    for i in [ x for x in range(12) if not x in namecolums ]:
-        b = b + (a[i] or '0') + ','
-        if i == xtkbdkey_index:
-            # RFB keycodes are XT kbd keycodes with a slightly
-            # different encoding of 0xe0 scan codes. RFB uses
-            # the high bit of the first byte, instead of the low
-            # bit of the second byte.
-            rfbkey = int(a[i] or '0')
-            rfbkey = (rfbkey & 0x100) >> 1 | (rfbkey & 0x7f)
-
-    # Append RFB keycode as the last column
-    b = b + str(rfbkey)
-    print "    { " + b + "},"
-
-print '};'
+    values = re.match("([^,]*)," * 13 + "([^,]*)$", line[0:-1]).groups()
+
+    data = []
+    for v in values:
+        data.append(v)
+
+    # RFB keycodes are XT kbd keycodes with a slightly
+    # different encoding of 0xe0 scan codes. RFB uses
+    # the high bit of the first byte, instead of the low
+    # bit of the second byte.
+    rfbkey = int(data[xtkbdkey_index] or '0')
+    rfbkey = (rfbkey & 0x100) >> 1 | (rfbkey & 0x7f)
+    data.append(rfbkey)
+
+    keycodes.append(data)
+    max = max + 1
+
+print "#define VIR_KEYMAP_ENTRY_MAX " + str(max)
+
+for i in range(len(cols)):
+    col=cols[i]
+    name=col[0]
+    isname=col[1]
+    if isname:
+        print "const char *virKeymapNames_" + name + "[] = {"
+    else:
+        print "unsigned short virKeymapValues_" + name + "[] = {"
+
+    for entry in keycodes:
+        if isname:
+            print "  " + quotestring(entry[i] or "NULL") + ","
+        else:
+            print "  " + (entry[i] or "0") + ","
+
+    print "};\n"
+
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index 7ce485c..9b2809d 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -22,51 +22,57 @@
 #include <string.h>
 #include <stddef.h>
 
-#define getfield(object, field_type, field_offset) \
-    (*(typeof(field_type) *)((char *)(object) + field_offset))
-
-struct keycode {
-    const char *linux_name;
-    const char *os_x_name;
-    const char *win32_name;
-    unsigned short linux_keycode;
-    unsigned short os_x;
-    unsigned short atset1;
-    unsigned short atset2;
-    unsigned short atset3;
-    unsigned short xt;
-    unsigned short xt_kbd;
-    unsigned short usb;
-    unsigned short win32;
-    unsigned short rfb;
-};
 
 #define VIRT_KEY_INTERNAL
 #include "virkeymaps.h"
 
-static unsigned int codeOffset[] = {
+static const char **virKeymapNames[] = {
     [VIR_KEYCODE_SET_LINUX] =
-        offsetof(struct keycode, linux_keycode),
+      virKeymapNames_linux,
     [VIR_KEYCODE_SET_XT] =
-        offsetof(struct keycode, xt),
+      NULL,
     [VIR_KEYCODE_SET_ATSET1] =
-        offsetof(struct keycode, atset1),
+      NULL,
     [VIR_KEYCODE_SET_ATSET2] =
-        offsetof(struct keycode, atset2),
+      NULL,
     [VIR_KEYCODE_SET_ATSET3] =
-        offsetof(struct keycode, atset3),
+      NULL,
     [VIR_KEYCODE_SET_OSX] =
-        offsetof(struct keycode, os_x),
+      virKeymapNames_os_x,
     [VIR_KEYCODE_SET_XT_KBD] =
-        offsetof(struct keycode, xt_kbd),
+      NULL,
     [VIR_KEYCODE_SET_USB] =
-        offsetof(struct keycode, usb),
+      NULL,
     [VIR_KEYCODE_SET_WIN32] =
-        offsetof(struct keycode, win32),
+      virKeymapNames_win32,
     [VIR_KEYCODE_SET_RFB] =
-        offsetof(struct keycode, rfb),
+      NULL,
 };
-verify(ARRAY_CARDINALITY(codeOffset) == VIR_KEYCODE_SET_LAST);
+verify(ARRAY_CARDINALITY(virKeymapNames) == VIR_KEYCODE_SET_LAST);
+
+static unsigned short *virKeymapValues[] = {
+    [VIR_KEYCODE_SET_LINUX] =
+      virKeymapValues_linux,
+    [VIR_KEYCODE_SET_XT] =
+      virKeymapValues_xt,
+    [VIR_KEYCODE_SET_ATSET1] =
+      virKeymapValues_atset1,
+    [VIR_KEYCODE_SET_ATSET2] =
+      virKeymapValues_atset2,
+    [VIR_KEYCODE_SET_ATSET3] =
+      virKeymapValues_atset3,
+    [VIR_KEYCODE_SET_OSX] =
+      virKeymapValues_os_x,
+    [VIR_KEYCODE_SET_XT_KBD] =
+      virKeymapValues_xt_kbd,
+    [VIR_KEYCODE_SET_USB] =
+      virKeymapValues_usb,
+    [VIR_KEYCODE_SET_WIN32] =
+      virKeymapValues_win32,
+    [VIR_KEYCODE_SET_RFB] =
+      virKeymapValues_rfb,
+};
+verify(ARRAY_CARDINALITY(virKeymapValues) == VIR_KEYCODE_SET_LAST);
 
 VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST,
     "linux",
@@ -81,68 +87,40 @@ VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST,
     "rfb",
 );
 
-static int __virKeycodeValueFromString(unsigned int name_offset,
-                                           unsigned int code_offset,
-                                           const char *keyname)
+int virKeycodeValueFromString(virKeycodeSet codeset,
+                              const char *keyname)
 {
     int i;
 
-    for (i = 0; i < ARRAY_CARDINALITY(virKeycodes); i++) {
-        const char *name = getfield(virKeycodes + i, const char *, name_offset);
+    for (i = 0; i < VIR_KEYMAP_ENTRY_MAX; i++) {
+        if (!virKeymapNames[codeset] ||
+            !virKeymapValues[codeset])
+            continue;
 
-        if (name && STREQ(name, keyname))
-            return getfield(virKeycodes + i, unsigned short, code_offset);
-    }
-
-    return -1;
-}
+        const char *name = virKeymapNames[codeset][i];
 
-int virKeycodeValueFromString(virKeycodeSet codeset, const char *keyname)
-{
-    switch (codeset) {
-    case VIR_KEYCODE_SET_LINUX:
-        return __virKeycodeValueFromString(offsetof(struct keycode, linux_name),
-                                           offsetof(struct keycode, linux_keycode),
-                                           keyname);
-    case VIR_KEYCODE_SET_OSX:
-        return __virKeycodeValueFromString(offsetof(struct keycode, os_x_name),
-                                           offsetof(struct keycode, os_x),
-                                           keyname);
-    case VIR_KEYCODE_SET_WIN32:
-        return __virKeycodeValueFromString(offsetof(struct keycode, win32_name),
-                                           offsetof(struct keycode, win32),
-                                           keyname);
-    default:
-        return -1;
-    }
-}
-
-static int __virKeycodeValueTranslate(unsigned int from_offset,
-                                      unsigned int to_offset,
-                                      int key_value)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_CARDINALITY(virKeycodes); i++) {
-        if (getfield(virKeycodes + i, unsigned short, from_offset) == key_value)
-            return getfield(virKeycodes + i, unsigned short, to_offset);
+        if (name && STREQ_NULLABLE(name, keyname))
+            return virKeymapValues[codeset][i];
     }
 
     return -1;
 }
 
+
 int virKeycodeValueTranslate(virKeycodeSet from_codeset,
                              virKeycodeSet to_codeset,
                              int key_value)
 {
-    if (key_value <= 0)
-        return -1;
+    int i;
 
-    key_value = __virKeycodeValueTranslate(codeOffset[from_codeset],
-                                           codeOffset[to_codeset],
-                                           key_value);
     if (key_value <= 0)
         return -1;
 
-    return key_value;
+
+    for (i = 0; i < VIR_KEYMAP_ENTRY_MAX; i++) {
+        if (virKeymapValues[from_codeset][i] == key_value)
+            return virKeymapValues[to_codeset][i];
+    }
+
+    return -1;
 }
-- 
1.8.1.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]