[PATCH 2/2] xkb: strdup the values returned by XkbGetRulesDflts

[Dan Nicholson]

On Tue, Apr 14, 2009 at 10:43 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> XkbGetRulesDftls may get a copy of what will later be freed when passed into
> XkbSetRulesDftls.
>
> On the second run of XkbGet/SetRulesDflts:
> XkbGetRulesDflts(rmlvo)
>        rmlvo->rules = current-rules
>
> XkbSetRulesDflts(rmlvo)
>        free(current-rules)
>        current-rules = strdup(rmlvo->rules)
>
> Leaving us with garbage in current-rules.
>
> This patch requires callers of XkbGetRulesDflts to free the associated memory.
>
> See also
> http://lists.freedesktop.org/archives/xorg-devel/2009-February/000305.html
>
> Reported-by: Benjamin Close <Benjamin.Close at clearchain.com>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Good call.

> ---
>  xkb/xkbInit.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/xkb/xkbInit.c b/xkb/xkbInit.c
> index 2e75612..9d4d9a2 100644
> --- a/xkb/xkbInit.c
> +++ b/xkb/xkbInit.c
> @@ -111,6 +111,10 @@ static Bool                XkbWantRulesProp=       XKB_DFLT_RULES_PROP;
>
>  /***====================================================================***/
>
> +/**
> + * Get the current default XKB rules.
> + * Caller must free the data in rmlvo.
> + */
>  void
>  XkbGetRulesDflts(XkbRMLVOSet *rmlvo)
>  {
> @@ -124,6 +128,12 @@ XkbGetRulesDflts(XkbRMLVOSet *rmlvo)
>     else               rmlvo->variant= XKB_DFLT_VARIANT;
>     if (XkbOptionsDflt)        rmlvo->options= XkbOptionsDflt;
>     else               rmlvo->options= XKB_DFLT_OPTIONS;
> +
> +    rmlvo->rules = strdup(rmlvo->rules);
> +    rmlvo->model = strdup(rmlvo->model);
> +    rmlvo->layout = strdup(rmlvo->layout);
> +    rmlvo->variant = strdup(rmlvo->variant);
> +    rmlvo->options = strdup(rmlvo->options);
>  }
>
>  static Bool
> @@ -586,6 +596,17 @@ InitKeyboardDeviceStruct(DeviceIntPtr dev, XkbRMLVOSet *rmlvo,
>     XkbSetRulesDflts(rmlvo);
>     XkbSetRulesUsed(rmlvo);
>
> +    if (rmlvo_dflts.rules)
> +        xfree(rmlvo_dflts.rules);
> +    if (rmlvo_dflts.model)
> +        xfree(rmlvo_dflts.model);
> +    if (rmlvo_dflts.layout)
> +        xfree(rmlvo_dflts.layout);
> +    if (rmlvo_dflts.variant)
> +        xfree(rmlvo_dflts.variant);
> +    if (rmlvo_dflts.options)
> +        xfree(rmlvo_dflts.options);

Can we get a helper for this to make it easier on callers? Something like:

XkbFreeRMLVO(XkbRMLVOSet *rmlvo, Bool freeRules)
{
    if (!rmlvo)
        return;

    xfree(rmlvo->rules);
    xfree(rmlvo->model);
    xfree(rmlvo->layout);
    xfree(rmlvo->variant);
    xfree(rmlvo->options);

    if (freeRules)
        xfree(rmlvo);
}

I think xfree/Xfree already does the right thing with NULL arguments.

--
Dan