Division by zero in TabDecorator CID 1210846 (easy)

[kallisti5]

Per CID 1210846, there is a potential division by zero...

​http://cgit.haiku-os.org/haiku/tree/src/servers/app/decorator/TabDecorator.cpp#n431

In TabDecorator::_DistributeTabSize(float delta)

int32 nTabsWithMaxSize = 0;
.
.
for (int32 i = 0; i < fTabList.CountItems(); i++) {
  (make nTabsWithMaxSize larger)
}
.
.
minus /= nTabsWithMaxSize;

I don't have the time to study the code and fix it, but it was a big enough issue I decided it needed a ticket. If fTabList.CountItems() is 0, the code will divide by 0

[jessicah]

Logically impossible for it to ever be 0. This is why it has an assert, rather than an if check. Adding an if check would hide a bug deeper in the code (and add an unnecessary branch). The CID should be set to not a bug.

[korli]

I would initialize nTabsWithMaxSize to 1 instead of 0, and remove the assignment line 424.

[kallisti5]

@jessicah Yeah, I saw the assertion check after opening this. However it's checking something other than what it's dividing. While that assertion checks the value in a round-about way, if anyone were to modify the code they could easily miss such a check.

Anyway, yeah... Since now it's more of a cosmetic thing, lowering the priority. I still think it is some low hanging fruit that could be corrected. @korli also had a good solution.

[sambuddhabasu1]

I want to work on this bug.

[pulkomandy]

This isn't a clean way to handle this. What I would do is:

• Store the value of CountTabs in a local variable (tabCount or so)
• ASSERT on the variable, then use it for the loop

This makes sure the assert and the loop use the same bound (it doesn't look like there is any locking, so in the current code a tab could be removed while the function runs?).

Then, I would also add a NULL check on the tab inside the loop (and skip the tab if it is NULL)

[jscipione]

Fixed in hrev48748