From: Pawel Wasowski Date: Fri, 16 Oct 2020 22:02:05 +0000 (+0200) Subject: [datacontrol] Fix a few issues X-Git-Tag: submit/tizen/20201104.133613~2^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=57c7d321930c4240226a571818eea40c4f13934a;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [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 --- diff --git a/src/datacontrol/datacontrol_instance.cc b/src/datacontrol/datacontrol_instance.cc index c0a6ba39..a7694c57 100644 --- a/src/datacontrol/datacontrol_instance.cc +++ b/src/datacontrol/datacontrol_instance.cc @@ -18,6 +18,7 @@ #include +#include #include #include #include @@ -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 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(); 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 valueString = value.get(); + const std::string& columnName = column.get(); + const std::string& valueString = value.get(); - 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(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 28167708..cb639f04 100644 --- 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);