[PATCH 2/7] xkb: add a call to init an XkbRMLVOSet from const chars
Hans de Goede
hdegoede at redhat.com
Mon Feb 3 02:12:44 PST 2014
Hi,
On 02/03/2014 01:07 AM, Peter Hutterer wrote:
> 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 :)
Ah right this is in a test-case, sorry I missed that, in that case, consider the
entire amended series:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
More information about the xorg-devel
mailing list