[haiku-commits] Re: r42080 - in haiku/trunk/src/apps/debugger: . model user_interface user_interface/gui user_interface/gui/inspector_window

On Fri, Jun 10, 2011 at 4:02 AM, Ingo Weinhold <ingo_weinhold@xxxxxx> wrote:
> The recursion can be avoided by simply reordering the code:
>        lookup
>        if existing
>                get reference
>                unless last reference already released, return
>                move to dead list
>        allocate new
>
> refCount isn't needed BTW.
>

Noted.

>
> A list should be fully sufficient for the dead blocks. It's a bit unfortunate
> that SinglyLinkedList requires a SinglyLinkedListLink. That has annoyed me
> more than once.
>

True, that was more or less laziness on my part, sorry.

> The base class version is guaranteed to only call delete, so that could be
> done directly as well. At any rate either the deletes in _RemoveBlock() or
> the one here needs to go. I'd vote for keeping the one here.

That's not the case though ; _RemoveBlock() is destroying the hash
table entry structures, not the blocks themselves. That having been
said, I do need to change it so the entries don't keep references to
the blocks or they'll prevent the blocks from ever being removed.

> It would be nicer from a design point of view to avoid the direct knowledge
> of TeamMemoryBlockManager in TeamMemoryBlock by introducing a
> TeamMemoryBlockOwner interface with a single callback.
>

i.e. something like:

class TeamMemoryBlockOwner {
public:
    void RemoveBlock(TeamMemoryBlock* block);
}

? Thanks for the review in any event, will deal with these issues
after work tonight.

Regards,

Rene

Other related posts: