[PATCH 2/7] xkb: add a call to init an XkbRMLVOSet from const chars

[Hans de Goede]

Hi,

On 01/31/2014 12:36 AM, Peter Hutterer wrote:
> On Thu, Jan 30, 2014 at 09:40:33AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 01/30/2014 12:51 AM, Peter Hutterer wrote:
>>> Just forcing everything to const char* is not helpful, compiler warnings are
>>> supposed to warn about broken code. Forcing everything to const when it
>>> clearly isn't less than ideal.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>   hw/xfree86/common/xf86Config.c | 16 ++++++++--------
>>>   include/xkbrules.h             | 10 +++++-----
>>>   include/xkbsrv.h               |  8 ++++++++
>>>   test/xkb.c                     | 16 +++++++++-------
>>>   xkb/xkbInit.c                  | 25 ++++++++++++++++++++-----
>>>   5 files changed, 50 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
>>> index 258b22b..542d5ab 100644
>>> --- a/hw/xfree86/common/xf86Config.c
>>> +++ b/hw/xfree86/common/xf86Config.c
>>> @@ -777,13 +777,7 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
>>>       MessageType from;
>>>       const char *s;
>>>       XkbRMLVOSet set;
>>> -
>>> -    /* Default options. */
>>> -    set.rules = "base";
>>> -    set.model = "pc105";
>>> -    set.layout = "us";
>>> -    set.variant = NULL;
>>> -    set.options = NULL;
>>> +    const char *rules;
>>>
>>>       /*
>>>        * Merge the ServerLayout and ServerFlags options.  The former have
>>> @@ -963,9 +957,15 @@ configServerFlags(XF86ConfFlagsPtr flagsconf, XF86OptionPtr layoutopts)
>>>        * evdev rules set. */
>>>   #if defined(linux)
>>>       if (!xf86Info.forceInputDevices)
>>> -        set.rules = "evdev";
>>> +        rules = "evdev";
>>> +    else
>>>   #endif
>>> +        rules = "base";
>>> +
>>> +    /* Xkb default options. */
>>> +    XkbInitRules(&set, rules, "pc105", "us", NULL, NULL);
>>>       XkbSetRulesDflts(&set);
>>> +    XkbFreeRMLVOSet(&set, FALSE);
>>>
>>>       xf86Info.useDefaultFontPath = TRUE;
>>>       xf86Info.useDefaultFontPathFrom = X_DEFAULT;
>>> diff --git a/include/xkbrules.h b/include/xkbrules.h
>>> index 956eade..ab5b4b2 100644
>>> --- a/include/xkbrules.h
>>> +++ b/include/xkbrules.h
>>> @@ -30,11 +30,11 @@
>>>   /***====================================================================***/
>>>
>>>   typedef struct _XkbRMLVOSet {
>>> -    const char *rules;
>>> -    const char *model;
>>> -    const char *layout;
>>> -    const char *variant;
>>> -    const char *options;
>>> +    char *rules;
>>> +    char *model;
>>> +    char *layout;
>>> +    char *variant;
>>> +    char *options;
>>>   } XkbRMLVOSet;
>>>
>>>   typedef struct _XkbRF_VarDefs {
>>> diff --git a/include/xkbsrv.h b/include/xkbsrv.h
>>> index 0b9ca06..e799799 100644
>>> --- a/include/xkbsrv.h
>>> +++ b/include/xkbsrv.h
>>> @@ -738,6 +738,14 @@ extern _X_EXPORT void XkbClearAllLatchesAndLocks(DeviceIntPtr /* dev */ ,
>>>                                                    XkbEventCausePtr       /* cause */
>>>       );
>>>
>>> +extern _X_EXPORT void XkbInitRules(XkbRMLVOSet * /* rmlvo   */,
>>> +                                   const char *  /* rules   */,
>>> +                                   const char *  /* model   */,
>>> +                                   const char *  /* layout  */,
>>> +                                   const char *  /* variant */,
>>> +                                   const char *  /* options */
>>> +    ) ;
>>> +
>>>   extern _X_EXPORT void XkbGetRulesDflts(XkbRMLVOSet *    /* rmlvo */
>>>       );
>>>
>>
>> Ack to all of the above.
>>
>>> diff --git a/test/xkb.c b/test/xkb.c
>>> index 955e72d..bfacc87 100644
>>> --- a/test/xkb.c
>>> +++ b/test/xkb.c
>>> @@ -82,15 +82,15 @@ xkb_get_rules_test(void)
>>>   static void
>>>   xkb_set_rules_test(void)
>>>   {
>>> -    XkbRMLVOSet rmlvo = {
>>> -        .rules = "test-rules",
>>> -        .model = "test-model",
>>> -        .layout = "test-layout",
>>> -        .variant = "test-variant",
>>> -        .options = "test-options"
>>> -    };
>>> +    XkbRMLVOSet rmlvo;
>>>       XkbRMLVOSet rmlvo_new = { NULL };
>>>
>>> +    rmlvo.rules = strdup("test-rules");
>>> +    rmlvo.model = strdup("test-model");
>>> +    rmlvo.layout = strdup("test-layout");
>>> +    rmlvo.variant = strdup("test-variant");
>>> +    rmlvo.options = strdup("test-options");
>>> +
>>>       XkbSetRulesDflts(&rmlvo);
>>>       XkbGetRulesDflts(&rmlvo_new);
>>>
>>> @@ -106,6 +106,8 @@ xkb_set_rules_test(void)
>>>       assert(strcmp(rmlvo.layout, rmlvo_new.layout) == 0);
>>>       assert(strcmp(rmlvo.variant, rmlvo_new.variant) == 0);
>>>       assert(strcmp(rmlvo.options, rmlvo_new.options) == 0);
>>> +
>>> +    XkbFreeRMLVOSet(&rmlvo, FALSE);
>>>   }
>>>
>>>   /**
>>
>> This should use the new XkbInitRules rather then diy strdup, so as
>> to be balanced wrt to the XkbFreeRMLVOSet call.
>
> amended, local diff is:
>
> diff --git a/test/xkb.c b/test/xkb.c
> index bfacc87..9047f59 100644
> --- a/test/xkb.c
> +++ b/test/xkb.c
> @@ -85,11 +85,13 @@ xkb_set_rules_test(void)
>       XkbRMLVOSet rmlvo;
>       XkbRMLVOSet rmlvo_new = { NULL };
>
> -    rmlvo.rules = strdup("test-rules");
> -    rmlvo.model = strdup("test-model");
> -    rmlvo.layout = strdup("test-layout");
> -    rmlvo.variant = strdup("test-variant");
> -    rmlvo.options = strdup("test-options");
> +    XkbInitRules(&rmlvo, "test-rules", "test-model", "test-layout",
> +                         "test-variant", "test-options");
> +    assert(rmlvo.rules);
> +    assert(rmlvo.model);
> +    assert(rmlvo.layout);
> +    assert(rmlvo.variant);
> +    assert(rmlvo.options);

Why the asserts? Now that you use XNFstrdup in XkbInitRules those are
not necessary anymore, right ?

Otherwise this looks good.

Regards,

Hans