[haiku-commits] Re: haiku: hrev48748 - src/servers/app/decorator
- From: Adrien Destugues <pulkomandy@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 30 Jan 2015 16:38:37 +0100
On Fri, Jan 30, 2015 at 10:26:46AM -0500, John Scipione wrote:
> On Friday, January 30, 2015, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:
>
> > Am 29/01/2015 um 22:40 schrieb jscipione@xxxxxxxxx:
> >
> >> 4ac21cd37da3: Issue 11111 - Division by zero in TabDecorator
> >>
> >
> > The patch does not solve or change the issue at all.
>
>
> The issue of division by 0 in 11111 was invalid so this represents code
> cleanup instead. The patch description could have been written more clearly.
There isn't any investigation showing that it is invalid. As I
mentionned in the ticket, there doesn't seem to be any locking in the
function so I assume the tab list could change while iterating it.
Either locking should be added, or the function should have a comment
explaining which lock should be held while calling it to avoid that
condition.
> >> + for (int32 i = 0; i < tabCount; i++) {
> >> Decorator::Tab* tab = fTabList.ItemAt(i);
> >> + if (tab == NULL)
> >> + continue;
> >>
> >
> > Why check for tab == NULL here? What problem should this solve?
> >
>
> See comment 8 on 11111. It solves the case where the tab list item is
> replaced with NULL (accidentally I hope).
It tries to handle the case where an element is removed from the list
while iterating it. Which should not happen if there is proper locking,
and cannot really be checked this way anyway.
--
Adrien.
Other related posts:
