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

Hans de Goede hdegoede at redhat.com
Thu Jan 30 00:40:33 PST 2014


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.

>  static void
>  XkbSetRulesUsed(XkbRMLVOSet * rmlvo)
>  {
> 

Regards,

Hans


More information about the xorg-devel mailing list