[haiku-commits] Re: haiku: hrev48748 - src/servers/app/decorator
- From: Jessica Hamilton <jessica.l.hamilton@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sat, 31 Jan 2015 04:40:05 +1300
On 31/01/2015 4:27 AM, "John Scipione" <jscipione@xxxxxxxxx> 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.
>
>>> +++ b/src/servers/app/decorator/TabDecorator.cpp
>>> @@ -406,13 +406,17 @@ TabDecorator::_DoTabLayout()
>>> void
>>> TabDecorator::_DistributeTabSize(float delta)
>>> {
>>> - ASSERT(CountTabs() > 1);
>>> + int32 tabCount = fTabList.CountItems();
>>> + ASSERT(tabCount > 1);
>>>
>>> float maxTabSize = 0;
>>> float secMaxTabSize = 0;
>>> int32 nTabsWithMaxSize = 0;
>>> - for (int32 i = 0; i < fTabList.CountItems(); i++) {
>>> + 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).
Checks for NULL for code that logically should never be NULL actually has
the negative consequence of hiding bugs. This is why asserts exist.
For example, if another thread is modifying the value, such a check like
this could potentially mask the need for locking.
Other related posts:
