Skip to content

Fix two low-hanging fruit compiler warnings #10442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 19, 2025

Conversation

jschmidt-icinga
Copy link
Contributor

This fixes two compiler warnings that are spamming the compile log on my system.

build/icinga2/lib/icinga/notification-ti.hpp:21:24: warning: ‘virtual icinga::String icinga::NotificationNameComposer::MakeName(const icinga::String&, const icinga::Object::Ptr&) const’ can be marked override [-Wsuggest-override]
   21 |         virtual String MakeName(const String& shortName, const Object::Ptr& context) const;
      |                        ^~~~~~~~
build/icinga2/lib/icinga/notification-ti.hpp:22:33: warning: ‘virtual icinga::Dictionary::Ptr icinga::NotificationNameComposer::ParseName(const icinga::String&) const’ can be marked override [-Wsuggest-override]
   22 |         virtual Dictionary::Ptr ParseName(const String& name) const;
      |                                 ^~~~~~~~~

This one affects the generated files but as the warning suggests, they should easily be able to be marked as override without any negative effects.

icinga2/lib/icinga/notification.cpp: In member function ‘void icinga::Notification::BeginExecuteNotification(icinga::NotificationType, const icinga::CheckResult::Ptr&, bool, bool, const icinga::String&, const icinga::String&)’:
icinga2/lib/icinga/notification.cpp:475:54: warning: enumerated mismatch in conditional expression: ‘icinga::ServiceState’ vs ‘icinga::HostState’ [-Wenum-compare]
  475 |                         uint_fast8_t state = service ? service->GetState() : host->GetState();
      |                                              ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
icinga2/lib/icinga/notification.cpp:504:54: warning: enumerated mismatch in conditional expression: ‘icinga::ServiceState’ vs ‘icinga::HostState’ [-Wenum-compare]
  504 |                         uint_fast8_t state = service ? service->GetState() : host->GetState();
      |                                              ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one stems from using the ternary operator to assign values of different types (HostState and ServiceState) to an integer or string. The solution is to first convert/cast the values, then assign them via the switch in the ternary operator.

@jschmidt-icinga jschmidt-icinga requested a review from yhabteab May 19, 2025 10:16
@cla-bot cla-bot bot added the cla/signed label May 19, 2025
@yhabteab yhabteab added the core/quality Improve code, libraries, algorithms, inline docs label May 19, 2025
@yhabteab yhabteab added this to the 2.15.0 milestone May 19, 2025
@jschmidt-icinga jschmidt-icinga force-pushed the jschmidt/fix-compiler-warnings branch from 5923e88 to f8d3bac Compare May 19, 2025 10:31
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm glad we're finally addressing compiler warnings! 🥳 Btw, here are more low-hanging fruits:

@yhabteab yhabteab enabled auto-merge May 19, 2025 10:48
@yhabteab yhabteab merged commit 44b3382 into master May 19, 2025
26 checks passed
@yhabteab yhabteab deleted the jschmidt/fix-compiler-warnings branch May 19, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed core/quality Improve code, libraries, algorithms, inline docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants