[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [PATCHv3 13/14] xfree86: fix bad free configInputDevices
From:       Dan Nicholson <dbn.lists () gmail ! com>
Date:       2011-03-31 12:41:35
Message-ID: AANLkTi=pSx5jOdLfTs1WH0K8OJ8Eay6eUYq3ybm89ge8 () mail ! gmail ! com
[Download RAW message or body]

On Thu, Mar 31, 2011 at 3:57 AM, Erkki Seppala <erkki.seppala@vincit.fi> wrote:
> On 30.03.2011 16:18, Dan Nicholson wrote:
>>
>> <tiago.vignatti@nokia.com>  wrote:
>>>
>>> diff --git a/hw/xfree86/common/xf86Config.c
>>> b/hw/xfree86/common/xf86Config.c
>>> index 7a23253..114bdc3 100644
>>> --- a/hw/xfree86/common/xf86Config.c
>>> +++ b/hw/xfree86/common/xf86Config.c
>>> @@ -1459,8 +1459,9 @@ configInputDevices(XF86ConfLayoutPtr layout,
>>> serverLayoutPtr servlayoutp)
>>>     while (irp) {
>>>        indp[count] = xf86AllocateInput();
>>>        if (!configInput(indp[count], irp->iref_inputdev, X_CONFIG)) {
>>> -           while(count--)
>>> +           do {
>>>                free(indp[count]);
>>> +           } while(count--);
>>>            free(indp);
>>>            return FALSE;
>>>        }
>>
>> Since the original code was doing a post-test decrement, wouldn't it
>> have wound all the way down to "free(indp[0])"? If it was "while
>> (--count)", I'd agree this is needed.
>
> If count = 1, the original code would wind up releasing only indp[0]. The
> revised code releases both indp[0] and indp[1], where indp[1] is allocated
> just a few lines above.

Oh, right that's what I was missing. With the corrected blame commit,

Reviewed-by: Dan Nicholson <dbn.lists@gmail.com>
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[prev in list] [next in list] [prev in thread] [next in thread]