On Thu, Feb 26, 2009 at 04:22:56PM +1030, Benjamin Close wrote:
> When allocating a second keyboard structure xkbGetRulesDflt
> is called to get the defaults for rmlvo.
> 
> With the second keyboard instance these defaults
> were the values previously allocated in the first call to
> XkbSetRulesDflt; rmlvo is then assigned this value.
> 
> rmlvo is then passed into InitKeyboardDeviceStruct which in turn
> calls xkbSetRulesDflt. xkbSetRulesDflts did:
> 
>     if( xkbRulesDflt )
>          _XkbFree(XkbRulesDflt);
>          XkbRulesDflt= (rmlvo->rules?_XkbDupString(rmlvo->rules):NULL);
> 
> Problem was by freeing XkbRulesDflt, rmlvo->rules was also freed
> hence the dup returned bogus data.
> 
> Fix this problem for both the Dflts and the Used cases.
> 
> Signed-off-by: Benjamin Close <[email protected]>

Here's what I had in mind. It doesn't fix the case where the caller can
free XKB internal data after calling XkbGetRulesDflts.

--
Dan

>From 27661caafa577d12380d7af1cc9101531fed3ee7 Mon Sep 17 00:00:00 2001
From: Dan Nicholson <[email protected]>
Date: Thu, 26 Feb 2009 16:14:01 -0800
Subject: [PATCH] xkb: Don't free strings the caller points to

Since XkbGetRulesDflts fills out a XkbRMLVOSet with pointers to internal
state, XkbSetRulesUsed and XkbSetRulesDflts need to be careful that they
do not subsequently free these pointers. Among other things, it means
the following _XkbDupString will copy garbage.

It might be better if XkbGetRulesDflts duped the internal strings for
the caller.

Signed-off-by: Dan Nicholson <[email protected]>
---
 xkb/xkbInit.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/xkb/xkbInit.c b/xkb/xkbInit.c
index 1f5f8dc..cee2532 100644
--- a/xkb/xkbInit.c
+++ b/xkb/xkbInit.c
@@ -193,19 +193,19 @@ char *                    pval;
 static void
 XkbSetRulesUsed(XkbRMLVOSet *rmlvo)
 {
-    if (XkbRulesUsed)
+    if (XkbRulesUsed != rmlvo->rules)
         _XkbFree(XkbRulesUsed);
     XkbRulesUsed= (rmlvo->rules?_XkbDupString(rmlvo->rules):NULL);
-    if (XkbModelUsed)
+    if (XkbModelUsed != rmlvo->model)
        _XkbFree(XkbModelUsed);
     XkbModelUsed= (rmlvo->model?_XkbDupString(rmlvo->model):NULL);
-    if (XkbLayoutUsed)
+    if (XkbLayoutUsed != rmlvo->layout)
        _XkbFree(XkbLayoutUsed);
     XkbLayoutUsed= (rmlvo->layout?_XkbDupString(rmlvo->layout):NULL);
-    if (XkbVariantUsed)
+    if (XkbVariantUsed != rmlvo->variant)
        _XkbFree(XkbVariantUsed);
     XkbVariantUsed= (rmlvo->variant?_XkbDupString(rmlvo->variant):NULL);
-    if (XkbOptionsUsed)
+    if (XkbOptionsUsed != rmlvo->options)
        _XkbFree(XkbOptionsUsed);
     XkbOptionsUsed= (rmlvo->options?_XkbDupString(rmlvo->options):NULL);
     if (XkbWantRulesProp)
@@ -217,27 +217,27 @@ void
 XkbSetRulesDflts(XkbRMLVOSet *rmlvo)
 {
     if (rmlvo->rules) {
-        if (XkbRulesDflt)
+        if (XkbRulesDflt != rmlvo->rules)
            _XkbFree(XkbRulesDflt);
         XkbRulesDflt= _XkbDupString(rmlvo->rules);
     }
     if (rmlvo->model) {
-       if (XkbModelDflt)
+        if (XkbModelDflt != rmlvo->model)
            _XkbFree(XkbModelDflt);
        XkbModelDflt= _XkbDupString(rmlvo->model);
     }
     if (rmlvo->layout) {
-       if (XkbLayoutDflt)
+        if (XkbLayoutDflt != rmlvo->layout)
            _XkbFree(XkbLayoutDflt);
        XkbLayoutDflt= _XkbDupString(rmlvo->layout);
     }
     if (rmlvo->variant) {
-       if (XkbVariantDflt)
+        if (XkbVariantDflt != rmlvo->variant)
            _XkbFree(XkbVariantDflt);
        XkbVariantDflt= _XkbDupString(rmlvo->variant);
     }
     if (rmlvo->options) {
-       if (XkbOptionsDflt)
+        if (XkbOptionsDflt != rmlvo->options)
            _XkbFree(XkbOptionsDflt);
        XkbOptionsDflt= _XkbDupString(rmlvo->options);
     }
-- 
1.5.6.6
_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to