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

[Peter Hutterer]

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);
 
     XkbSetRulesDflts(&rmlvo);
     XkbGetRulesDflts(&rmlvo_new);


 
> > 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.

amended, thanks.

Cheers,
   Peter