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

[Nate Gallaher]

James Hawkins wrote:
> 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.
>
>   
I am not seeing a way to do this which incorporates the ability to track 
externally allocated memory (like that of msi_dup_property). How do you 
propose to avoid a lookup during cond_free to check if the pointer given 
is from an external allocation?

~Nate