Silent error caused by implicit conversion of SmartPointer to integer

[leekillough]

Bug Report

A SmartPointer is implicitly converted to an integer, causing undefined behavior.

In message_helper.cc line 952, the first argument should be an integer App ID, but instead is the smart pointer itself implicitly converted to an integer:

 LOG4CXX_AUTO_TRACE(logger_);
   if (app) {
-    SendSetAppIcon(app, app->app_icon_path(), app_man);
+    SendSetAppIcon(app->app_id(), app->app_icon_path(), app_man);
     SendGlobalPropertiesToHMI(app, app_man);
     SendShowRequestToHMI(app, app_man);
   }

Fix submitted: #1828

Cc: @madhuy @davidswi @fronneburg

[aderiabin]

Contributor priority is set Low with reason: This is specific compilation issue.

[leekillough]

The bug is not compiler-specific, nor specific to any "compilation issue". It is a silent bug in the original C++ code, caused by the implicit conversion of SmartPtr objects to bool and int.

In porting the SDL source to C++11, implementing most of https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0043-upgrade-c++-standard.md, and thus being able to use explicit user-defined conversion functions instead of implicit ones (just like std::shared_ptr doesn't allow implicit conversion to bool), this bug was detected and fixed.

It is very trivial to fix, and I thought it should be pointed out.

It has already been fixed at Xevo. You must have very high standards to rate Xevo that low of a contributor!!!

To see the difference, compile this code:

struct unsafe {
  unsafe() {}
  operator bool() const { return 0; }
};

struct safe {
  safe() {}
  explicit // C++11
  operator bool() const { return 0; }
};

int main() {
  unsafe u;
  int iu = u;
  safe s;
  int is = s;
}

sdl_core # g++ -std=c++11 test.cc
test.cc: In function ‘int main()’:
test.cc:16:12: error: cannot convert ‘safe’ to ‘int’ in initialization
   int is = s;
            ^

As you can see, the unsafe class, which has a non-explicit conversion to bool, can be implicitly converted to int in the int iu = u; declaration, exactly like the SharedPtr-derived argument app to SendSetAppIcon can be implicitly converted to the uint32_t app_id parameter. However, the safe class, whose conversion to bool is explicit, cannot be implicitly converted to int, as cannot the C++11 std::shared_ptr class.

In the buggy original implementation, the app_id received by SendSetAppIcon is always 1, rather than the true id that the app->app_id() accessor returns.

[leekillough]

Fixed

[jacobkeeler]

@leekillough When was this fixed? Your PR is still open, and the change hasn't been merged elsewhere.

[jacobkeeler]

Fixed with the merge of #1828