The International Simutrans Forum

 

Author Topic: Fixes for more compiler warnings  (Read 333 times)

0 Members and 1 Guest are viewing this topic.

Offline ceeac

  • *
  • Posts: 27
Fixes for more compiler warnings
« on: March 12, 2019, 10:27:48 AM »
This patch fixes some more compiler warnings, mostly related to unused variables and functions, but also warnings reported by Wclass-memaccess and Wtautological-constant-out-of-range-compare.
The full commit log is available here: https://github.com/ceeac/simutrans/compare/master...ceeac:less-warnings

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: Fixes for more compiler warnings
« Reply #1 on: March 12, 2019, 05:05:48 PM »
Other reviewers might find https://stackoverflow.com/questions/45349079/how-to-use-attribute-fallthrough-correctly-in-gcc interesting.

I'm not sure why a decimal constant was changed into a hexidecimal one for motd loading. Neither makes it clear why the value is this strange value.

I don't think putting the return value of fread into len is correct given the way fread is called. The return value will be either 0 or 1, since it returns the number of objects read, not the number of bytes. Swap the second and third argument, and it will be correct.

Why was a lot of UTF-8 related functions just commented out? If we don't need them, there should be no harm in removing them completely. If it turns out that we need them later after all, they can either be found in the file history, or just reimplemented based on UTF-8 specification. They are not that complicated.

In binary_heap_tpl, would it perhaps be better to initialize the member variable in an initializer list?

Online Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4555
  • Languages: EN, DE, AT
Re: Fixes for more compiler warnings
« Reply #2 on: March 12, 2019, 09:11:30 PM »
the change to remove unused work_cost in simhalt.cc looks more like a bug, i.e., this workcost has to be used instead of cost?

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9418
  • Languages: De,EN,JP
Re: Fixes for more compiler warnings
« Reply #3 on: March 13, 2019, 06:17:32 AM »
I wonder why using decimals instead of hexadecimals gives a warning. That compiler seems to be broken for me ...

The workcost is indeed not booked. Changed in r8718

Also why is everyone so bent on fixing warnings for GCC? There are more compiler out there. MSVC emits more than 100 warnings, although not all useful. A lot about are loosing precisions due to cast; something GCC is awful silent about.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: Fixes for more compiler warnings
« Reply #4 on: March 13, 2019, 06:30:18 AM »
Also why is everyone so bent on fixing warnings for GCC?

The right question might be why MSVC users are not bent on fixing warnings. Maybe the warnings are not as visible. Although my sample size is small, I am more alert to warnings that come when I use command-line tools to compile than when compiling inside an IDE.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9418
  • Languages: De,EN,JP
Re: Fixes for more compiler warnings
« Reply #5 on: March 13, 2019, 08:23:50 AM »
Without being too cynical: Many GCC users seems to have rather extreme standpoints (like OpenSource licenses commata) and thus are also very much bent on achieving zero warning level skills. Ignoring that Oracle, Digital Mars, Windriver, Microsoft, Intel may give different warnings, since those are not standartised.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: Fixes for more compiler warnings
« Reply #6 on: March 13, 2019, 03:43:44 PM »
Well, projects were this is noticeable are probably libraries, frameworks or servers where bugs quickly become a serious vulnerability to their users. Without a standardized way of suppressing warnings that the developer has checked out as not a problem, the easiest way to get rid of them is to rewrite the code so that the warnings no longer trigger. If warnings are just allowed to accumulate, people stop paying attention to the new ones, as they are a needle in a stack of needles.

Online Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4555
  • Languages: EN, DE, AT
Re: Fixes for more compiler warnings
« Reply #7 on: March 21, 2019, 04:47:17 PM »
@prissi: there are still unused variables which point to missing accounting of making stuff public.
Code: [Select]
simhalt.cc:2362:22: warning: unused variable 'woowner' [-Wunused-variable]
                                        player_t *const woowner = wo->get_owner();
                                                        ^
simhalt.cc:2366:19: warning: unused variable 'workcost' [-Wunused-variable]
                                        sint64 const workcost = -welt->scale_with_month_length(cost * welt->get_settings().cst_make_public_months);
                                                     ^
simhalt.cc:2353:13: warning: unused variable 'wo' [-Wunused-variable]
                wayobj_t *wo = NULL;

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9418
  • Languages: De,EN,JP
Re: Fixes for more compiler warnings
« Reply #8 on: March 25, 2019, 02:02:16 PM »
Thank you, removed in r8739