[haiku-commits] Re: haiku: hrev48748 - src/servers/app/decorator
- From: Adrien Destugues <pulkomandy@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 30 Jan 2015 17:09:36 +0100
On Fri, Jan 30, 2015 at 10:54:33AM -0500, John Scipione wrote:
> So what is the solution here? The assert only checks that the list count is
> > 0. What if the list has a NULL hole in it as in:
>
> Tab 1
> Tab 2
> NULL
> Tab3
>
> ??
This should be checked when inserting items to the list, not when
iterating it. This way the bug is detected immediately instead of
at an unrelated point in the code.
If all insertions are checked against this, and it still happens, it
means the list is gettting cuorrupted. In that case, it's better to
crash (and having no NULL check will do just that).
>
> > For example, if another thread is modifying the value, such a check like
> > this could potentially mask the need for locking.
> >
> I wasn't really thinking about thread locking issues although that is
> another good point. Would a solution that locks access to the list before
> modifying it be overkill here?
I mentionned threading issues in the ticket already. I will repeat what
I said in my message:
- If this function must only be called with some lock held, that should
be documented in a comment for the function. This way we know what to
expect. It could also check that the lock is actually held with an
assert (for example BView does a lot of "looper must be locked"
checks and will call debugger in case of misuses).
- If this function can be called without any lock held, then we must
review the code to see if the list can be accessed by multiple
threads. If so, we must add locking.
It's not possible to decide wether locking should be added without
further study of the code.
--
Adrien.
Other related posts:
