[datacontrol] Fix a few issues

This commit:
- adds freeing of the result passed to MAPGetResponseCallback
TODO: after fixing this problem, the app crashes. It was reported to
native developers and we need to wait for their answer, which will tell
us, if the result should be freed or not
- removes unused DatacontrolInstance::DataControlManagerGetConsumer
method
- adds handling errors of bundle_create() and bundle_add_str()
- adds the code destroying handles created in
DatacontrolInstance::Create{MAP, SQL}Handle() methods in case of
failures
- adds missing "const" and/or "&" in one of the functions to unify the
code style in the module
- fixes a grammar issue of one log message

[Verification] tct-datacontrol-tizen-tests: 100% pass rate

Change-Id: If524bc4f733762658a07993e39b8eaa9fe59d56d
Signed-off-by: Pawel Wasowski <p.wasowski2@samsung.com>

diff --git a/src/datacontrol/datacontrol_instance.cc b/src/datacontrol/datacontrol_instance.cc
index c0a6ba3..a7694c5 100644 (file)
--- a/src/datacontrol/datacontrol_instance.cc
+++ b/src/datacontrol/datacontrol_instance.cc
@@ -18,6 +18,7 @@
 
 #include <glib.h>
 
+#include <tizen.h>
 #include <algorithm>
 #include <functional>
 #include <limits>
@@ -98,7 +99,6 @@ DatacontrolInstance::DatacontrolInstance() {
   REGISTER_METHOD(SQLDataControlConsumerRemove);
   REGISTER_METHOD(MappedDataControlConsumerRemoveValue);
   REGISTER_METHOD(MappedDataControlConsumerUpdateValue);
-  REGISTER_METHOD(DataControlManagerGetConsumer);
   REGISTER_METHOD(SQLDataControlConsumerInsert);
   REGISTER_METHOD(MappedDataControlConsumerGetValue);
   REGISTER_METHOD(DataControlConsumerObjectRemoveChangeListener);
@@ -269,6 +269,23 @@ static void MAPGetResponseCallback(int requestId, data_control_h handle, char**
                                    void* user_data) {
   ScopeLogger();
 
+  /*
+   * According to the documentation of data_control_map_get_response_cb, result_value_list
+   * should be freed with the code, commented out below.
+   * However, when we free result_value_list, the app crashes.
+   *
+   * We have asked data-control developers about this problem and we are waiting for the
+   * response, telling us wether we should free the list or not.
+   *
+   * TODO (when we get the answer): comment out the SCOPE_EXIT below or remove it from the code.
+   */
+  /*SCOPE_EXIT {
+    for (int i = 0; i < result_value_count; ++i) {
+      free(result_value_list[i]);
+    }
+    free(result_value_list);
+  };*/
+
   std::lock_guard<std::mutex> lock(IdMapMutex);
   DatacontrolInformation* info = IdMap[requestId];
   if (info == NULL) {
@@ -516,14 +533,6 @@ int DatacontrolInstance::RunSQLDataControlJob(const std::string& providerId,
     return;                                                                      \
   }
 
-void DatacontrolInstance::DataControlManagerGetConsumer(const picojson::value& args,
-                                                        picojson::object& out) {
-  ScopeLogger();
-  CHECK_PRIVILEGE_ACCESS(kPrivilegeDatacontrol, &out);
-
-  CHECK_EXIST(args, "providerId", out)
-  CHECK_EXIST(args, "dataId", out)
-}
 void DatacontrolInstance::SQLDataControlConsumerInsert(const picojson::value& args,
                                                        picojson::object& out) {
   ScopeLogger();
@@ -563,6 +572,12 @@ void DatacontrolInstance::SQLDataControlConsumerInsert(const picojson::value& ar
         picojson::array::size_type size = std::min(columnsLength, valuesLength);
 
         bundle* b = ::bundle_create();
+        if (!b) {
+          auto error_code = get_last_result();
+          LoggerE("bundle_create() failed: %s", get_error_message(error_code));
+          return error_code;
+        }
+
         SCOPE_EXIT {
           bundle_free(b);
         };
@@ -620,7 +635,7 @@ void DatacontrolInstance::SQLDataControlConsumerUpdate(const picojson::value& ar
   picojson::object updateData = args.get("updateData").get<picojson::object>();
 
   if (!updateData.count("columns") || !updateData.count("values")) {
-    LogAndReportError(TypeMismatchException("columns and values is required updateData argument"),
+    LogAndReportError(TypeMismatchException("columns and values are required updateData arguments"),
                       out);
     return;
   }
@@ -643,6 +658,12 @@ void DatacontrolInstance::SQLDataControlConsumerUpdate(const picojson::value& ar
         picojson::array::size_type size = std::min(columnsLength, valuesLength);
 
         bundle* b = ::bundle_create();
+        if (!b) {
+          auto error_code = get_last_result();
+          LoggerE("bundle_create() failed: %s", get_error_message(error_code));
+          return error_code;
+        }
+
         SCOPE_EXIT {
           bundle_free(b);
         };
@@ -654,10 +675,13 @@ void DatacontrolInstance::SQLDataControlConsumerUpdate(const picojson::value& ar
             break;
           }
 
-          std::string& columnName = column.get<std::string>();
-          std::string valueString = value.get<std::string>();
+          const std::string& columnName = column.get<std::string>();
+          const std::string& valueString = value.get<std::string>();
 
-          bundle_add_str(b, columnName.c_str(), valueString.c_str());
+          int result = bundle_add_str(b, columnName.c_str(), valueString.c_str());
+          if (BUNDLE_ERROR_NONE != result) {
+            return result;
+          }
         }
 
         return ::data_control_sql_update(handle, b, where.c_str(), requestId);
@@ -949,10 +973,20 @@ int DatacontrolInstance::CreateMAPHandle(const std::string& providerId, const st
   RETURN_IF_FAIL(result, "Creating map data control handle is failed with error");
 
   result = ::data_control_map_set_provider_id(*handle, providerId.c_str());
-  RETURN_IF_FAIL(result, "Setting provider id is failed with error");
+  if (result != DATA_CONTROL_ERROR_NONE) {
+    LoggerE("Setting provider id is failed with error : %s", ::get_error_message(result));
+    data_control_map_destroy(*handle);
+    *handle = nullptr;
+    return result;
+  }
 
   result = ::data_control_map_set_data_id(*handle, dataId.c_str());
-  RETURN_IF_FAIL(result, "Setting data id is failed the error");
+  if (result != DATA_CONTROL_ERROR_NONE) {
+    LoggerE("Setting data id is failed with error : %s", ::get_error_message(result));
+    data_control_map_destroy(*handle);
+    *handle = nullptr;
+    return result;
+  }
 
   return result;
 }
@@ -966,10 +1000,20 @@ int DatacontrolInstance::CreateSQLHandle(const std::string& providerId, const st
   RETURN_IF_FAIL(result, "Creating sql data control handle is failed with error");
 
   result = ::data_control_sql_set_provider_id(*handle, providerId.c_str());
-  RETURN_IF_FAIL(result, "Setting provider id is failed with error");
+  if (result != DATA_CONTROL_ERROR_NONE) {
+    LoggerE("Setting provider id is failed with error : %s", ::get_error_message(result));
+    data_control_sql_destroy(*handle);
+    *handle = nullptr;
+    return result;
+  }
 
   result = ::data_control_sql_set_data_id(*handle, dataId.c_str());
-  RETURN_IF_FAIL(result, "Setting data id is failed the error");
+  if (result != DATA_CONTROL_ERROR_NONE) {
+    LoggerE("Setting data id is failed with error : %s", ::get_error_message(result));
+    data_control_sql_destroy(*handle);
+    *handle = nullptr;
+    return result;
+  }
 
   return result;
 }
@@ -1100,7 +1144,7 @@ void DatacontrolInstance::callback(data_control_h provider, data_control_data_ch
 // result_callback method is used only for pass information to errorCallback if any error
 // occur while adding listener
 void DatacontrolInstance::result_callback(data_control_h provider, data_control_error_e result,
-                                          int callback_id, void* user_data) {
+                                          int watch_id, void* user_data) {
   ScopeLogger();
   if (DATA_CONTROL_ERROR_NONE != result) {
     auto data = static_cast<ReplyCallbackData*>(user_data);
@@ -1111,8 +1155,8 @@ void DatacontrolInstance::result_callback(data_control_h provider, data_control_
     // handled earlier
     LogAndReportError(IOException("DataControlConsumerObjectAddChangeListener failed"), obj);
     Instance::PostMessage(data->_instance, event.serialize().c_str());
-    if (0 != callback_id) {
-      data->_instance->EraseMap(callback_id);
+    if (0 != watch_id) {
+      data->_instance->EraseMap(watch_id);
     }
   }
 }

diff --git a/src/datacontrol/datacontrol_instance.h b/src/datacontrol/datacontrol_instance.h
index 2816770..cb639f0 100644 (file)
--- a/src/datacontrol/datacontrol_instance.h
+++ b/src/datacontrol/datacontrol_instance.h
@@ -70,7 +70,6 @@ class DatacontrolInstance : public common::ParsedInstance {
                                                    std::string* type);
 
  private:
-  void DataControlManagerGetConsumer(const picojson::value& args, picojson::object& out);
   void SQLDataControlConsumerInsert(const picojson::value& args, picojson::object& out);
   void SQLDataControlConsumerUpdate(const picojson::value& args, picojson::object& out);
   void SQLDataControlConsumerRemove(const picojson::value& args, picojson::object& out);