The International Simutrans Forum

 

Author Topic: Fix for crash when resizing window when scroll band is displayed  (Read 644 times)

0 Members and 1 Guest are viewing this topic.

Offline ceeac

  • Devotee
  • *
  • Posts: 256
This patch fixes a crash when resizing the main window to a very small width or height while the scroll band is displayed.

Offline kierongreen

  • Dev Team, Coder/patcher
  • Devotee
  • *
  • Posts: 2346
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #1 on: July 09, 2020, 11:17:23 AM »
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?

Offline ceeac

  • Devotee
  • *
  • Posts: 256
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #2 on: July 09, 2020, 02:02:51 PM »
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.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10663
  • Languages: De,EN,JP
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #3 on: July 10, 2020, 06:58:58 AM »
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.

Offline kierongreen

  • Dev Team, Coder/patcher
  • Devotee
  • *
  • Posts: 2346
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #4 on: July 10, 2020, 08:48:14 AM »
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....

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 10663
  • Languages: De,EN,JP
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #5 on: July 10, 2020, 02:03:04 PM »
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.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4877
  • Languages: EN, DE, AT
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #6 on: July 10, 2020, 02:13:28 PM »
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.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5691
  • Languages: EN, NO
Re: Fix for crash when resizing window when scroll band is displayed
« Reply #7 on: July 12, 2020, 09:59:29 AM »
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.