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