Wireshark mailing list archives

Re: [PATCH] Re: Freeing memory of se_alloc'ated object


From: Max Dmitrichenko <dmitrmax () gmail com>
Date: Tue, 3 May 2011 04:55:02 +0400

2011/5/3 ronnie sahlberg <ronniesahlberg () gmail com>:
I think registering a destructor for an allocated is very useful, but
it would be very uncommon.
Most allocations never need a destructur, so it shouldnt be made
mandatory in the allocation functions.

As it is implemented now, it's not mandatory - just a branch of already
existing functions.

So instead of creating a new API for allocations with a destructor I
would suggest adding a new _add_destructor call instead.
That way you can add destructors to existing objects after they are
allocated, post allocation time.

Ok. I see you, guys, like the idea of dynamic feature management :)
Both ways have right to exist, but this one is more error prone.
Copy-pasting the code can add two destructors call to the same object.

something like :

   object = se_alloc(...);
   se_set_emem_destructor(object,  void (*your_callback)(void
*object), void *private_data);


Then this can just add the destructor to a linked list of
struct se_destructors {
   struct se_destructors *next;
   void *object;
   void *private_data;
   void (*callback)(void *object, void *private_data);
};

Well, it was the first idea that came up when I started to implement this.
But when I looked into emem's code, I found the other way to implement
this. Generally I have no objections but one - read below.

I threw a private_data pointer in there too.
Once you have callbacks you almost always enbd up wanting to pass data
like this to it too.
While it is very common to use such private_data pointers, when talking
about callbacks, it is generally a bad design practice if the object being
destructed needs some external context.

While you've made a statement that destructor's will be rarely used among
all the Wireshark code, the feature of private_data will be rarely used among
destructors' code ;)

Anyway, the feature introduced in my patch can be reimplemented. But
may I ask the dumb question? Who decides here what is going to the trunk
and what it should like?

I'm ready to (re-)implement any design of this feature but need some
concentrated piece of absolute wisdom from local Guru :)

--
  Max
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

Current thread: