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 <[email protected]> > >>>--- > >>> 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
