James Hawkins wrote:
On Mon, Jan 4, 2010 at 7:05 AM, Nate Gallaher <[email protected]> wrote:
James Hawkins wrote:
On Sat, Jan 2, 2010 at 10:36 AM, Nathan Gallaher
<[email protected]> 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
