[PATCH 1/2] msi: queue dynamically allocated strings in cond.y
Nate Gallaher
ngallaher at deepthought.org
Tue Jan 5 11:11:27 CST 2010
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
More information about the wine-devel
mailing list