Re: [PATCH v3] xen/keyhandler: Rework keyhandler infrastructure

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/09/15 12:58, Jan Beulich wrote:
>>>> On 24.09.15 at 13:47, <andrew.cooper3@citrix.com> wrote:
>> On 24/09/15 12:46, Jan Beulich wrote:
>>>>>> On 24.09.15 at 13:05, <andrew.cooper3@citrix.com> wrote:
>>>> +static struct keyhandler {
>>>> +    union {
>>>> +        keyhandler_fn_t *fn;
>>>> +        irq_keyhandler_fn_t *irq_fn;
>>>> +    };
>>>> +
>>>> +    const char *desc;    /* Description for help message.                 
>> */
>>>> +    bool_t irq_callback, /* Call in irq context? if not, tasklet context. 
>> */
>>>> +        diagnostic;      /* Include in 'dump all' handler.                
>> */
>>>> +} key_table[128] __read_mostly =
>>>> +{
>>>> +#define KEYHANDLER(k, f, desc, diag)            \
>>>> +    [k] = { .fn = (f), desc, 0, diag }
>>>> +
>>>> +#define IRQ_KEYHANDLER(k, f, desc, diag)        \
>>>> +    [k] = { .irq_fn = (f), desc, 1, diag }
>>> I'm sorry for noticing only now, but I'm afraid these (looking as
>>> odd - but correct - as the other ones did) won't build with older
>>> gcc either.
>> Urgh yes.  I think we might have to just explicitly typecast the
>> pointer, because .irq_fn is uninitialised in older GCC.  Would that be ok?
> Yes, certainly better than re-adding the u name. Another
> alternative would be a void * as the first union alternative,
> but I suppose future gcc might become more strict about
> conversions between function and data pointers...

This appears to work for me.

diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 7d168e9..8c43d85 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -43,10 +43,10 @@
 } key_table[128] __read_mostly =
 {
 #define KEYHANDLER(k, f, desc, diag)            \
-    [k] = { .fn = (f), desc, 0, diag }
+    [k] = { { f }, desc, 0, diag }
 
 #define IRQ_KEYHANDLER(k, f, desc, diag)        \
-    [k] = { .irq_fn = (f), desc, 1, diag }
+    [k] = { { (keyhandler_fn_t*)f }, desc, 1, diag }
 
     IRQ_KEYHANDLER('A', do_toggle_alt_key, "toggle alternative key
handling", 0),
     IRQ_KEYHANDLER('d', dump_registers, "dump registers", 1),