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

[Hans de Goede]

Hi,

On 01/30/2014 09:40 AM, 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.
> 
>> diff --git a/xkb/xkbInit.c b/xkb/xkbInit.c
>> index 22b971f..c0b85ce 100644
>> --- a/xkb/xkbInit.c
>> +++ b/xkb/xkbInit.c
>> @@ -129,11 +129,11 @@ XkbFreeRMLVOSet(XkbRMLVOSet * rmlvo, Bool freeRMLVO)
>>      if (!rmlvo)
>>          return;
>>  
>> -    free((void *) rmlvo->rules);
>> -    free((void *) rmlvo->model);
>> -    free((void *) rmlvo->layout);
>> -    free((void *) rmlvo->variant);
>> -    free((void *) rmlvo->options);
>> +    free(rmlvo->rules);
>> +    free(rmlvo->model);
>> +    free(rmlvo->layout);
>> +    free(rmlvo->variant);
>> +    free(rmlvo->options);
>>  
>>      if (freeRMLVO)
>>          free(rmlvo);
> 
> Ack.
> 
>> @@ -206,6 +206,21 @@ XkbWriteRulesProp(ClientPtr client, void *closure)
>>      return TRUE;
>>  }
>>  
>> +void
>> +XkbInitRules(XkbRMLVOSet *rmlvo,
>> +             const char *rules,
>> +             const char *model,
>> +             const char *layout,
>> +             const char *variant,
>> +             const char *options)
>> +{
>> +    rmlvo->rules = rules ? strdup(rules) : NULL;
>> +    rmlvo->model = model ? strdup(model) : NULL;
>> +    rmlvo->layout = layout ? strdup(layout) : NULL;
>> +    rmlvo->variant = variant ? strdup(variant) : NULL;
>> +    rmlvo->options = options ? strdup(options) : NULL;
>> +}
>> +
> 
> void, so use XNFstrdup please. I know we should never see OOM, but if we
> do it is better to fail then to continue silently.

p.s. with the above changes this is:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans