[PATCH] xkb: when copying the keymap, make sure the structs default to 0/NULL.
Daniel Stone
daniel at fooishbar.org
Thu Feb 7 00:10:40 PST 2008
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>
More information about the xorg
mailing list