From d8332a3d7c8b9d6c64b3a016fd6aa9b331a28a7b Mon Sep 17 00:00:00 2001 From: Piotr Kosko Date: Tue, 27 Mar 2018 14:59:02 +0200 Subject: [PATCH] [common] Coverity issues fix [Feature] Fixed coverity issues with below ids: 110914, 110006, 108656, 107626, 107387, 107077, 106006, 108993, 108297, 106912 Additionally fixed potential crash in download API - in case of native function failure, the "success" response is not trying to be build. [Verification] 100% passrate for modules: application, bluetooth, download, exif, filesystem, messaging-* Change-Id: I8d9482501a7375962408186feaa5ecd1f0826b1d Signed-off-by: Piotr Kosko --- src/application/application_manager.cc | 37 +++++++++++----- src/bluetooth/bluetooth_gatt_service.cc | 2 +- src/bluetooth/bluetooth_health_channel.cc | 2 +- src/bluetooth/bluetooth_socket.cc | 2 +- src/common/filesystem/filesystem_storage.h | 1 + src/download/download_instance.cc | 27 +++++++++--- src/exif/exif_instance.cc | 1 + src/exif/jpeg_file.cc | 4 +- src/messaging/email_manager.cc | 67 ++++++++--------------------- src/messaging/message.cc | 12 ++++-- src/messaging/messaging_manager.cc | 3 +- src/secureelement/secureelement_instance.cc | 2 +- 12 files changed, 84 insertions(+), 76 deletions(-) diff --git a/src/application/application_manager.cc b/src/application/application_manager.cc index 8744c0c..821f0fd 100644 --- a/src/application/application_manager.cc +++ b/src/application/application_manager.cc @@ -1385,20 +1385,37 @@ void ApplicationManager::StartAppInfoEventListener(picojson::object* out) { return; } +#define CHECK_APPLICATION_EVENT_ERROR(result, function_name) \ + if (PKGMGR_R_OK > result) { \ + StopAppInfoEventListener(); \ + LogAndReportError( \ + PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to register listener."), out, \ + ("Function %s failed: %s (%d)", function_name, get_error_message(result), result)); \ + return; \ + } + g_application_list_changed_broker.AddApplicationInstance(&instance_); - pkgmgr_client_set_status_type(pkgmgr_client_handle_, PACKAGE_MANAGER_STATUS_TYPE_INSTALL | - PACKAGE_MANAGER_STATUS_TYPE_UPGRADE); - pkgmgr_client_set_status_type(pkgmgr_client_uninstall_handle_, - PACKAGE_MANAGER_STATUS_TYPE_UNINSTALL); + int result = pkgmgr_client_set_status_type( + pkgmgr_client_handle_, + PACKAGE_MANAGER_STATUS_TYPE_INSTALL | PACKAGE_MANAGER_STATUS_TYPE_UPGRADE); + CHECK_APPLICATION_EVENT_ERROR(result, "pkgmgr_client_set_status_type") + + result = pkgmgr_client_set_status_type(pkgmgr_client_uninstall_handle_, + PACKAGE_MANAGER_STATUS_TYPE_UNINSTALL); + CHECK_APPLICATION_EVENT_ERROR(result, "pkgmgr_client_set_status_type") + + result = pkgmgr_client_listen_status(pkgmgr_client_handle_, + ApplicationListChangedBroker::ClientStatusListener, + &g_application_list_changed_broker); + CHECK_APPLICATION_EVENT_ERROR(result, "pkgmgr_client_listen_status") - pkgmgr_client_listen_status(pkgmgr_client_handle_, - ApplicationListChangedBroker::ClientStatusListener, - &g_application_list_changed_broker); - pkgmgr_client_listen_status(pkgmgr_client_uninstall_handle_, - ApplicationListChangedBroker::AppUninstallListener, - &g_application_list_changed_broker); + result = pkgmgr_client_listen_status(pkgmgr_client_uninstall_handle_, + ApplicationListChangedBroker::AppUninstallListener, + &g_application_list_changed_broker); + CHECK_APPLICATION_EVENT_ERROR(result, "pkgmgr_client_listen_status") +#undef CHECK_APPLICATION_EVENT_ERROR } else { LoggerD("Broker callback is already registered."); } diff --git a/src/bluetooth/bluetooth_gatt_service.cc b/src/bluetooth/bluetooth_gatt_service.cc index f6798a8..1200a7b 100644 --- a/src/bluetooth/bluetooth_gatt_service.cc +++ b/src/bluetooth/bluetooth_gatt_service.cc @@ -424,7 +424,7 @@ void BluetoothGATTService::WriteValue(const picojson::value& args, picojson::obj int value_size = value_array.size(); std::unique_ptr value_data(new char[value_size]); for (int i = 0; i < value_size; ++i) { - value_data[i] = (int) value_array[i].get(); + value_data[i] = (int)value_array[i].get(); } struct Data { diff --git a/src/bluetooth/bluetooth_health_channel.cc b/src/bluetooth/bluetooth_health_channel.cc index e119693..90d805d 100644 --- a/src/bluetooth/bluetooth_health_channel.cc +++ b/src/bluetooth/bluetooth_health_channel.cc @@ -77,7 +77,7 @@ void BluetoothHealthChannel::SendData(const picojson::value& data, picojson::obj std::unique_ptr data_ptr{new char[data_size]}; for (std::size_t i = 0; i < data_size; ++i) { - data_ptr[i] = (int) binary_data[i].get(); + data_ptr[i] = (int)binary_data[i].get(); } int ntv_ret = bt_hdp_send_data(channel, data_ptr.get(), data_size); diff --git a/src/bluetooth/bluetooth_socket.cc b/src/bluetooth/bluetooth_socket.cc index fea9ed1..a044a37 100644 --- a/src/bluetooth/bluetooth_socket.cc +++ b/src/bluetooth/bluetooth_socket.cc @@ -63,7 +63,7 @@ void BluetoothSocket::WriteData(const picojson::value& data, picojson::object& o std::unique_ptr data_ptr{new char[data_size]}; for (std::size_t i = 0; i < data_size; ++i) { - data_ptr[i] = (int) binary_data[i].get(); + data_ptr[i] = (int)binary_data[i].get(); } if (kBluetoothError == bt_socket_send_data(socket, data_ptr.get(), data_size)) { diff --git a/src/common/filesystem/filesystem_storage.h b/src/common/filesystem/filesystem_storage.h index 8ff7828..fd5dc17 100644 --- a/src/common/filesystem/filesystem_storage.h +++ b/src/common/filesystem/filesystem_storage.h @@ -79,6 +79,7 @@ class Storage : public VirtualRoot { int id() const { return id_; } + private: int id_; }; diff --git a/src/download/download_instance.cc b/src/download/download_instance.cc index fda4c57..4e1fa39 100644 --- a/src/download/download_instance.cc +++ b/src/download/download_instance.cc @@ -314,12 +314,11 @@ gboolean DownloadInstance::OnFinished(void* user_data) { LoggerW("%s", get_error_message(ret)); } out["status"] = picojson::value("completed"); + out["fullPath"] = + picojson::value(common::FilesystemProvider::Create().GetVirtualPath(fullPath)); } - out["callbackId"] = picojson::value(static_cast(callback_id)); - out["fullPath"] = picojson::value(common::FilesystemProvider::Create().GetVirtualPath(fullPath)); - Instance::PostMessage(downCbPtr->instance, picojson::value(out).serialize().c_str()); // downCbPtr is freed in destructor, it prevent from crash if OnFinished state // was called after OnCanceled or OnFailed @@ -504,8 +503,17 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso system_info_get_platform_bool("http://tizen.org/feature/network.telephony", &cell_support); system_info_get_platform_bool("http://tizen.org/feature/network.wifi", &wifi_support); +#define CHECK_CONNECTION_ERROR(ret) \ + if (CONNECTION_ERROR_NONE != ret) { \ + LogAndReportError( \ + common::PlatformResult(common::ErrorCode::UNKNOWN_ERR, "Connection problem occured"), \ + &out, ("Connection type is disconnected")); \ + return; \ + } + connection_h connection = nullptr; - connection_create(&connection); + ret = connection_create(&connection); + CHECK_CONNECTION_ERROR(ret) SCOPE_EXIT { connection_destroy(connection); }; @@ -513,11 +521,16 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso connection_cellular_state_e cell_state = CONNECTION_CELLULAR_STATE_OUT_OF_SERVICE; connection_wifi_state_e wifi_state = CONNECTION_WIFI_STATE_DEACTIVATED; - connection_get_cellular_state(connection, &cell_state); - connection_get_wifi_state(connection, &wifi_state); + ret = connection_get_cellular_state(connection, &cell_state); + CHECK_CONNECTION_ERROR(ret) + ret = connection_get_wifi_state(connection, &wifi_state); + CHECK_CONNECTION_ERROR(ret) connection_type_e connection_type = CONNECTION_TYPE_DISCONNECTED; - connection_get_type(connection, &connection_type); + ret = connection_get_type(connection, &connection_type); + CHECK_CONNECTION_ERROR(ret) + +#undef CHECK_CONNECTION_ERROR if (CONNECTION_TYPE_DISCONNECTED == connection_type) { LogAndReportError( diff --git a/src/exif/exif_instance.cc b/src/exif/exif_instance.cc index 121390c..0687d70 100644 --- a/src/exif/exif_instance.cc +++ b/src/exif/exif_instance.cc @@ -184,6 +184,7 @@ void ExifInstance::ExifManagerGetThumbnail(const picojson::value& args, picojson gchar* ch_uri = g_base64_encode(exif_data->data, exif_data->size); exif_data_unref(exif_data); std::string base64 = "data:image/" + ext + ";base64," + ch_uri; + g_free(ch_uri); std::pair pair; pair = std::make_pair("src", picojson::value(base64)); diff --git a/src/exif/jpeg_file.cc b/src/exif/jpeg_file.cc index e0640e1..aefd179 100644 --- a/src/exif/jpeg_file.cc +++ b/src/exif/jpeg_file.cc @@ -584,9 +584,9 @@ PlatformResult JpegFile::saveToFilePriv(const std::string& out_path) { std::unique_ptr exif_output_data; unsigned int exif_output_size = 0; - if (cur_marker != JPEG_MARKER_SOI && cur_marker != JPEG_MARKER_EOI) { + if (JPEG_MARKER_SOI != cur_marker && JPEG_MARKER_EOI != cur_marker) { unsigned short section_size = 2; - if (JPEG_MARKER_APP1 && cur->exif_data) { + if (JPEG_MARKER_APP1 == cur_marker && cur->exif_data) { unsigned char* tmp = NULL; exif_data_save_data(cur->exif_data, &tmp, &exif_output_size); if (!tmp || 0 == exif_output_size) { diff --git a/src/messaging/email_manager.cc b/src/messaging/email_manager.cc index 9a80c0d..256fbb3 100644 --- a/src/messaging/email_manager.cc +++ b/src/messaging/email_manager.cc @@ -175,6 +175,24 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< ScopeLogger(); email_mail_data_t* mail_data = NULL; email_mail_data_t* mail_data_final = NULL; + email_mailbox_t* mailbox_data = NULL; + email_account_t* account = NULL; + + SCOPE_EXIT { + if (mail_data_final) { + email_free_mail_data(&mail_data_final, 1); + } + if (mail_data) { + email_free_mail_data(&mail_data, 1); + } + if (mailbox_data) { + email_free_mailbox(&mailbox_data, 1); + } + if (account) { + email_free_account(&account, 1); + } + }; + int err = EMAIL_ERROR_NONE; PlatformResult ret = Message::convertPlatformEmail(message, &mail_data); @@ -183,13 +201,8 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< mail_data->account_id = account_id; // Adding "from" email address - email_account_t* account = NULL; err = email_get_account(account_id, GET_FULL_DATA_WITHOUT_PASSWORD, &account); if (EMAIL_ERROR_NONE != err) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Cannot retrieve email account information", ("email_get_account error: %d (%s)", err, get_error_message(err))); } @@ -200,18 +213,10 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< ss >> address_from; mail_data->full_address_from = strdup(address_from.c_str()); LoggerE("FROM %s", mail_data->full_address_from); - err = email_free_account(&account, 1); - if (EMAIL_ERROR_NONE != err) { - LoggerE("Failed to free account data memory"); - } + // Setting mailbox id - email_mailbox_t* mailbox_data = NULL; err = email_get_mailbox_by_mailbox_type(account_id, mailbox_type, &mailbox_data); if (EMAIL_ERROR_NONE != err) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return LogAndCreateResult( ErrorCode::UNKNOWN_ERR, "Cannot retrieve draft mailbox", ("email_get_mailbox_by_mailbox_type error: %d (%s)", err, get_error_message(err))); @@ -228,14 +233,6 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< // adding email without attachments err = email_add_mail(mail_data, NULL, 0, NULL, 0); if (EMAIL_ERROR_NONE != err) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } - ntv_ret = email_free_mailbox(&mailbox_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to destroy mailbox: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Couldn't add message to draft mailbox", ("email_add_mail error: %d (%s)", err, get_error_message(err))); } else { @@ -250,46 +247,20 @@ PlatformResult EmailManager::addMessagePlatform(int account_id, std::shared_ptr< if (message->getHasAttachment()) { ret = Message::addEmailAttachments(message); if (ret.IsError()) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return ret; } } err = email_get_mail_data(message->getId(), &mail_data_final); if (EMAIL_ERROR_NONE != err) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Couldn't retrieve added mail data", ("email_get_mail_data error: %d (%s)", err, get_error_message(err))); } ret = message->updateEmailMessage(*mail_data_final); if (ret.IsError()) { - int ntv_ret = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != ntv_ret) { - LoggerE("Failed to free mail data memory: %d (%s)", ntv_ret, get_error_message(ntv_ret)); - } return ret; } - err = email_free_mail_data(&mail_data_final, 1); - if (EMAIL_ERROR_NONE != err) { - LoggerE("Failed to free mail data final memory: %d (%s)", err, get_error_message(err)); - } - - err = email_free_mail_data(&mail_data, 1); - if (EMAIL_ERROR_NONE != err) { - LoggerE("Failed to free mail data memory: %d (%s)", err, get_error_message(err)); - } - - err = email_free_mailbox(&mailbox_data, 1); - if (EMAIL_ERROR_NONE != err) { - LoggerE("Failed to destroy mailbox: %d (%s)", err, get_error_message(err)); - } return PlatformResult(ErrorCode::NO_ERROR); } diff --git a/src/messaging/message.cc b/src/messaging/message.cc index d439357..ba6aaa6 100644 --- a/src/messaging/message.cc +++ b/src/messaging/message.cc @@ -338,15 +338,19 @@ std::string Message::convertEmailRecipients(const std::vector& reci PlatformResult saveToTempFile(const std::string& data, std::string* file_name) { ScopeLogger(); - char buf[] = "XXXXXX"; + char buf[] = "/tmp/XXXXXX"; mode_t mask = umask(S_IWGRP | S_IWOTH); - mkstemp(buf); // Just generate unique name + int file_descriptor = mkstemp(buf); + // Generate unique name and open the file descriptor + if (-1 == file_descriptor) { + return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Temp file creation failed"); + } - std::string tmp_name = std::string("/tmp/") + buf; + std::string tmp_name = buf; mode_t old_mask = umask(mask); - FILE* file = fopen(tmp_name.c_str(), "w"); + FILE* file = fdopen(file_descriptor, "w+"); umask(old_mask); if (NULL == file) { diff --git a/src/messaging/messaging_manager.cc b/src/messaging/messaging_manager.cc index dbb4f4c..566f198 100644 --- a/src/messaging/messaging_manager.cc +++ b/src/messaging/messaging_manager.cc @@ -55,7 +55,8 @@ MsgManagerCallbackData::MsgManagerCallbackData(MessagingInstance& instance) ScopeLogger(); } -MessagingManager::MessagingManager(MessagingInstance& instance) : instance_(instance) { +MessagingManager::MessagingManager(MessagingInstance& instance) + : m_msg_handle(nullptr), instance_(instance) { ScopeLogger(); int ret = msg_open_msg_handle(&m_msg_handle); if (ret != MSG_SUCCESS) { diff --git a/src/secureelement/secureelement_instance.cc b/src/secureelement/secureelement_instance.cc index 7313d70..eccb692 100644 --- a/src/secureelement/secureelement_instance.cc +++ b/src/secureelement/secureelement_instance.cc @@ -554,7 +554,7 @@ TizenResult SecureElementInstance::Transmit(picojson::object const& args, }; for (size_t i = 0; i < v_cmd_size; i++) { - cmd[i] = (int) v_cmd[i].get(); + cmd[i] = (int)v_cmd[i].get(); } int length = 0; -- 2.7.4