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

[Peter Hutterer]

On Fri, Jan 31, 2014 at 05:30:58PM +0100, Hans de Goede wrote:
> 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 ?

it's not necessary, but this is a test for the API.
Since we're now using a new call, the assert() serves to make sure that what
we get back is useful and doesn't just hand us a null-ed out struct. Could
be more extensive (strcmp for the input values) but there's a limit to how
detailed we need to go here :)

Cheers,
   Peter