Fix for crash when resizing window when scroll band is displayed

[ceeac]

This patch fixes a crash when resizing the main window to a very small width or height while the scroll band is displayed.

[kierongreen]

I realise that there may be various stylistic (or syntax) preferences but std::max and std::min are used twice each at present in simutrans whereas min and max are used many, many times. Is there a particular reason std:: versions are used here?

[ceeac]

From here:

IMO the std::min/max should completely replace the dangerous Simutrans defined ones. Far too easy for "bad things" happening with them  (compiler silently passing uint32 as sint32 and....)

I want to replace min/max by std::min/max eventually so this is one less occurrence to take care of.

[prissi]

Thank you for spotting this error. I submitted it, but using standard min and max. Because "std::min<KOORD_VAL>" is more text than the rest, so it confuses rather than making things clear. All types involved are in the end integer types, so why the cast? I think C is getting more and more broken.

[kierongreen]

Thanks prissi and agree. If someone wants to make that change to std versions throughout trunk they can as a separate patch which can be considered on its own merits.


It's not that the language is more broken it's that sometimes the way it had been implemented causes undefined behaviours. There has been greater focus in recent times on eliminating this to reduce bugs, particularly hard to trace ones.


For example comparing a signed char value -1 with an unsigned char value 255 might be processed as equal. Therefore, depending on how data is processed -2 might be evaluated as less than or greater than 255.


There are so many comparisons made throughout simutrans between signed and unsigned and different sizes of variable that tackling all these would be a massive piece of work. In at least one place having signed and unsigned variables as parameters (ribi or slope) is used to have different methods for a function in a way that not exactly transparent....

[prissi]

You can compile with MSVC, it gives signed unsigned warnings since 14 years. However, they are silenced, since 95% of them are code that is ok to run.

[Dwachs]

I do not  know if this is only me, but I do get exactly 2 warnings on signed-unsigned comparisons in code that is relatively recent.

[Ters]

Every new version of GCC adds a new bunch of new warnings. For mistakes that have possibly cost someone millions of euro.

std::min/max should not require explicit typing if the types are correct to begin with. What might be a problem is KOORD_VAL a = 1, b; b = std::min(a, 0); since a is a short and 0 is an int. Oddly, C does not seem to have a suffix for designating a number as short literal the way L can be used for longs and F for floats. C++11 allows defining custom suffixes, allowing std:min(a, 0_h);, which is shorter than having to write the full type either either as a template argument or a cast.