Re: [PATCH obexd 1/6 v2] Make use of g_slist_free_full when elements are dynamically-allocated

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 28, 2011 at 8:46 AM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi,
>
> On Thu, Jul 28, 2011 at 8:57 AM, Slawomir Bochenski <lkslawek@xxxxxxxxx> wrote:
>> Hello Luiz!
>>
>> On Wed, Jul 27, 2011 at 1:22 PM, Luiz Augusto von Dentz
>> <luiz.dentz@xxxxxxxxx> wrote:
>>> Hi Marcel,
>>>
>>> On Tue, Jul 19, 2011 at 12:04 PM, Luiz Augusto von Dentz
>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>> This would work for whatever file that includes config.h which seems
>>>> to be the perfect place for wrappers like this, but it doesn't seems
>>>> common to have this type of macros inside configure.ac/config.h so
>>>> perhaps it is a bad idea.
>>>
>>> So it seems you didn't like the idea, so shall I create a
>>> glib-helper.h that implements g_slist_free_full? In the other hand we
>>> have to remember to always include this file when using glib functions
>>> that have wrappers, so Im not sure if in the end this make sense since
>>> it might complicates things a bit too much for so little.
>>
>> I've seen that the glib team has gone with your proposal for
>> g_[s]list_free_full. As I had doubts about this change that I had
>> mentioned to you before, I did benchmark it and I've discussed results
>> with Matthias Classen from glib (who originally committed your
>> proposed changes). Actually version with loop is slower than what was
>> there before, both in single- and multithreaded applications. Changes
>> in glib have been already reverted.
>
> Not sure if you guys actually read my comments on bug
> https://bugzilla.gnome.org/show_bug.cgi?id=653935, I knew it has
> performance impact, although it could be used to prevent callback to
> remove the items causing g_slist_free to access invalid memory. Btw,
> our wrapper version is exactly the same os original glib does,
> g_slist_foreach + g_slist_free. (see BlueZ glib-helper.h)
>

But where in this bug report did you mention anything about that?
Besides one should not modify list from inside destroy function. It
gets direct pointer to data and it should not try to reverse find list
item for that. If you try to traverse list there you can always break
g_slist_free_full(), even after changes (by e.g. freeing next
element).

>> So as the g_slist_free_full() will be still just wrapping those two
>> functions we are calling now, is there a need for complicating things
>> that much instead of postponing using this until it is decided to drop
>> compatibility with glib versions w/o g_slist_free_full()?
>
> Exactly my point, so now you can stop arguing, ok.
>
> --
> Luiz Augusto von Dentz
>

-- 
Slawomir Bochenski
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux