[PATCH] xkb: when copying the keymap, make sure the structs default to 0/NULL.

[Daniel Stone]

On Thu, Feb 07, 2008 at 04:03:07PM +1030, Peter Hutterer wrote:
> Finally found the bug. Turns out that when copying keymaps, parts of 
> structs remain unintialised. this is a bad idea if you're trying to free 
> various pointers lateron (in my case dst->geom->sections->doodads).

Hi,
My general understanding is that you wouldn't try to free foo if sz_foo
was 0.  Of course, this being XKB, there's a lot of superfluous crap
that turns out to be unused, and the result makes no sense. \o/

> +            dst->geom->shapes = NULL;
>               dst->geom->num_shapes = 0;

Please no tab damage. :)

>                   tmp = xalloc(src->geom->num_sections * 
> sizeof(XkbSectionRec));
>               if (!tmp)
>                   return FALSE;
> +            memset(tmp, 0, src->geom->num_sections * 
> sizeof(XkbSectionRec));

Looks fine to me?

> +                } else
> +                    dsection->doodads = NULL;

Please use:
}
else {
    foo;
}

for consistency.

>               if (dst->geom->sz_key_aliases && dst->geom->key_aliases) {
>                   xfree(dst->geom->key_aliases);
> -                dst->geom->key_aliases = NULL;
>               }

If we should unconditionally free non-NULL pointers despite sz_foo (god
this makes less and less sense every time I type it), then we should
probably just test for key_aliases here, not sz_key_aliases.

>               if (dst->geom->label_font) {
>                   xfree(dst->geom->label_font);
> -                dst->geom->label_font = NULL;
>               }
> +            dst->geom->label_font = NULL;

This hunk has no effect.

Otherwise looks good though, thanks for tracking this down.

Cheers,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg/attachments/20080207/4b31e76e/attachment.pgp>