[PATCH 1/2] msi: queue dynamically allocated strings in cond.y

[James Hawkins]

On Mon, Jan 4, 2010 at 7:05 AM, Nate Gallaher <ngallaher at deepthought.org> wrote:
> James Hawkins wrote:
>>
>> On Sat, Jan 2, 2010 at 10:36 AM, Nathan Gallaher
>> <ngallaher at deepthought.org> wrote:
>>
>>>
>>>
>>
>>
>> +struct cond_mem {
>> +    struct list entry;
>> +    void *ptr;
>> +};
>>
>>
>> +
>> +static void cond_free( void *info, void *ptr )
>> +{
>> +    COND_input *cond = (COND_input*) info;
>> +    struct cond_mem *mem, *safety;
>> +
>> +    LIST_FOR_EACH_ENTRY_SAFE( mem, safety, &cond->mem, struct cond_mem,
>> entry )
>> +    {
>> +        if( mem->ptr == ptr )
>> +        {
>> +            msi_free( mem->ptr );
>> +            list_remove( &(mem->entry) );
>> +            msi_free( mem );
>> +            return;
>> +        }
>> +    }
>> +    ERR("Error freeing %p\n", ptr);
>> +}
>>
>>
>> This won't fly.  cond_free needs to be an O(1) operation, like your
>> original patch.
>>
>>
>
> This was exactly the same as your proposed implementation.
>
> Well, the simplest thing to do here is to no-op the cond_free function and
> just free all memory when parsing is done, but you seemed to not like that
> the last time.  Perhaps you were just looking for a solution that keeps the
> notion of a cond_free intact, with the implementation open?
>
> The only other alternative that I see here is to use a hashmap, but I don't
> immediately see an implementation of a proper hashmap available in
> include/wine.  I could go implement a hashmap, but I want to make sure
> that's the approach that you will accept before I do a bunch of work that
> you're not happy with again.
>
> Is there anything else that you talked with AJ about that I should know
> before I go further?  It would have been nice if whatever conversation there
> was had been shared publicly on the wine-devel list, since I could have
> avoided this headache.
>

My solution was wrong, which is why it wasn't committed.  The correct
solution is a mix between our two original patches.  You allocate the
list entry and the string data in one chunk of memory that you can
free all at once (your original patch did this).  In cond_free, you
remove the entry from the list and free the allocation.  At the end of
parsing, you loop through the list, and preferable throw a WARN up
that we've leaked each allocation that is left.  Ideally, this only
happens when the parse fails and the WARN can be ignored.  Keep in
mind you have to add the data from msi_dup_property to the allocation
list.

-- 
James Hawkins