[Download] Fixed unexpected result of fixing coverity issues

[Bug] When running on TV device, checking status of cellular network
  returned not supported error, the checking status of cellular needed
  to be moved into correct section instead of running this code in common
  code part.

[Verification] TCT result - 100%

Change-Id: I6cb171034588b2616222bde4101a278c491fdc52
Signed-off-by: Piotr Kosko <p.kosko@samsung.com>

diff --git a/src/download/download_instance.cc b/src/download/download_instance.cc
index 4e1fa39..33e2d7a 100644 (file)
--- a/src/download/download_instance.cc
+++ b/src/download/download_instance.cc
@@ -462,115 +462,136 @@ void DownloadInstance::progress_changed_cb(int download_id, long long unsigned r
   g_idle_add(OnProgressChanged, downCbPtr);
 }
 
+#define CHECK_CONNECTION_ERROR(ret)                                                            \
+  switch (ret) {                                                                               \
+    case CONNECTION_ERROR_NONE:                                                                \
+      break;                                                                                   \
+    case CONNECTION_ERROR_NOT_SUPPORTED:                                                       \
+      return LogAndCreateResult(                                                               \
+          common::ErrorCode::NOT_SUPPORTED_ERR,                                                \
+          "The networkType of the given DownloadRequest is not supported",                     \
+          ("The networkType of the given DownloadRequest is not supported"));                  \
+    default:                                                                                   \
+      return LogAndCreateResult(common::ErrorCode::UNKNOWN_ERR, "Connection problem occurred", \
+                                ("Connection problem occurred"));                              \
+  }
+
+common::PlatformResult DownloadInstance::CheckNetworkConnection(const std::string& network_type,
+                                                                bool& network_support,
+                                                                bool& network_available,
+                                                                DownloadInfoPtr di_ptr) {
+  connection_h connection = nullptr;
+  int ret = connection_create(&connection);
+  CHECK_CONNECTION_ERROR(ret)
+  SCOPE_EXIT {
+    connection_destroy(connection);
+  };
+
+  connection_type_e connection_type = CONNECTION_TYPE_DISCONNECTED;
+  ret = connection_get_type(connection, &connection_type);
+  CHECK_CONNECTION_ERROR(ret)
+
+  if (CONNECTION_TYPE_DISCONNECTED == connection_type) {
+    return LogAndCreateResult(common::ErrorCode::UNKNOWN_ERR, "Connection problem occurred",
+                              ("Connection type is disconnected"));
+  }
+
+  if ("CELLULAR" == network_type) {
+    // check if connection type is supported
+    bool cell_support = false;
+    system_info_get_platform_bool("http://tizen.org/feature/network.telephony", &cell_support);
+
+    // check if connection is available
+    connection_cellular_state_e cell_state = CONNECTION_CELLULAR_STATE_OUT_OF_SERVICE;
+    ret = connection_get_cellular_state(connection, &cell_state);
+    CHECK_CONNECTION_ERROR(ret)
+    bool cell_available = (CONNECTION_CELLULAR_STATE_CONNECTED == cell_state);
+
+    // setup download parameters using cellular network
+    network_support = cell_support;
+    network_available = cell_available;
+    di_ptr->network_type = DOWNLOAD_NETWORK_DATA_NETWORK;
+  } else if ("WIFI" == network_type) {
+    // check if connection type is supported
+    bool wifi_support = false;
+    system_info_get_platform_bool("http://tizen.org/feature/network.wifi", &wifi_support);
+
+    // check if connection is available
+    connection_wifi_state_e wifi_state = CONNECTION_WIFI_STATE_DEACTIVATED;
+    ret = connection_get_wifi_state(connection, &wifi_state);
+    CHECK_CONNECTION_ERROR(ret)
+    bool wifi_available = (CONNECTION_WIFI_STATE_CONNECTED == wifi_state);
+
+    // setup download parameters using wifi network
+    network_support = wifi_support;
+    network_available = wifi_available;
+    di_ptr->network_type = DOWNLOAD_NETWORK_WIFI;
+  } else if (("ALL" == network_type) && (CONNECTION_TYPE_DISCONNECTED != connection_type)) {
+    // setup download parameters using 'all' network
+    network_support = true;
+    network_available = true;
+    di_ptr->network_type = DOWNLOAD_NETWORK_ALL;
+  } else {
+    return LogAndCreateResult(
+        common::ErrorCode::INVALID_VALUES_ERR,
+        "The input parameter contains an invalid network type.",
+        ("The input parameter contains an invalid network type: %s", network_type.c_str()));
+  }
+  return common::PlatformResult(common::ErrorCode::NO_ERROR);
+}
+#undef CHECK_CONNECTION_ERROR
+
 void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojson::object& out) {
   ScopeLogger();
   CHECK_PRIVILEGE_ACCESS(kPrivilegeDownload, &out);
   CHECK_EXIST(args, "callbackId", out)
   CHECK_EXIST(args, "url", out)
 
-  int ret;
-  std::string networkType;
+  std::string network_type;
 
-  DownloadInfoPtr diPtr(new DownloadInfo);
+  DownloadInfoPtr di_ptr(new DownloadInfo);
 
-  diPtr->callbackId = static_cast<int>(args.get("callbackId").get<double>());
-  diPtr->url = args.get("url").is<std::string>() ? args.get("url").get<std::string>() : "";
+  di_ptr->callbackId = static_cast<int>(args.get("callbackId").get<double>());
+  di_ptr->url = args.get("url").is<std::string>() ? args.get("url").get<std::string>() : "";
 
   if (!args.get("destination").is<picojson::null>()) {
     if (args.get("destination").is<std::string>() &&
         args.get("destination").get<std::string>() != "") {
-      diPtr->destination = args.get("destination").get<std::string>();
-      diPtr->destination = common::FilesystemProvider::Create().GetRealPath(diPtr->destination);
+      di_ptr->destination = args.get("destination").get<std::string>();
+      di_ptr->destination = common::FilesystemProvider::Create().GetRealPath(di_ptr->destination);
     }
   }
 
   if (!args.get("fileName").is<picojson::null>()) {
     if (args.get("fileName").is<std::string>() && args.get("fileName").get<std::string>() != "") {
-      diPtr->file_name = args.get("fileName").get<std::string>();
+      di_ptr->file_name = args.get("fileName").get<std::string>();
     }
   }
 
   if (!args.get("networkType").is<picojson::null>()) {
-    networkType = args.get("networkType").is<std::string>()
-                      ? args.get("networkType").get<std::string>()
-                      : "ALL";
+    network_type = args.get("networkType").is<std::string>()
+                       ? args.get("networkType").get<std::string>()
+                       : "ALL";
   }
 
   bool network_support = false;
-  bool cell_support = false;
-  bool wifi_support = false;
-
-  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;
-  ret = connection_create(&connection);
-  CHECK_CONNECTION_ERROR(ret)
-  SCOPE_EXIT {
-    connection_destroy(connection);
-  };
-
-  connection_cellular_state_e cell_state = CONNECTION_CELLULAR_STATE_OUT_OF_SERVICE;
-  connection_wifi_state_e wifi_state = CONNECTION_WIFI_STATE_DEACTIVATED;
-
-  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;
-  ret = connection_get_type(connection, &connection_type);
-  CHECK_CONNECTION_ERROR(ret)
-
-#undef CHECK_CONNECTION_ERROR
-
-  if (CONNECTION_TYPE_DISCONNECTED == connection_type) {
-    LogAndReportError(
-        common::PlatformResult(common::ErrorCode::UNKNOWN_ERR, "Connection problem occured"), &out,
-        ("Connection type is disconnected"));
-    return;
-  }
-
   bool network_available = false;
-  bool cell_available = (CONNECTION_CELLULAR_STATE_CONNECTED == cell_state);
-  bool wifi_available = (CONNECTION_WIFI_STATE_CONNECTED == wifi_state);
-
-  if ("CELLULAR" == networkType) {
-    network_support = cell_support;
-    network_available = cell_available;
-    diPtr->network_type = DOWNLOAD_NETWORK_DATA_NETWORK;
-  } else if ("WIFI" == networkType) {
-    network_support = wifi_support;
-    network_available = wifi_available;
-    diPtr->network_type = DOWNLOAD_NETWORK_WIFI;
-  } else if (("ALL" == networkType) && (CONNECTION_TYPE_DISCONNECTED != connection_type)) {
-    network_support = true;
-    network_available = true;
-    diPtr->network_type = DOWNLOAD_NETWORK_ALL;
-  } else {
-    LogAndReportError(
-        common::PlatformResult(common::ErrorCode::INVALID_VALUES_ERR,
-                               "The input parameter contains an invalid network type."),
-        &out, ("The input parameter contains an invalid network type: %s", networkType.c_str()));
+  common::PlatformResult result =
+      CheckNetworkConnection(network_type, network_support, network_available, di_ptr);
+  if (!result) {
+    LogAndReportError(result, &out);
     return;
   }
 
   /*
    * There is no relevant feature for networkType == "ALL".
    */
-  if (!network_support && ("ALL" != networkType)) {
+  if (!network_support && ("ALL" != network_type)) {
     LogAndReportError(common::PlatformResult(common::ErrorCode::NOT_SUPPORTED_ERR,
                                              "The networkType of the given DownloadRequest "
                                              "is not supported on this device."),
-                      &out, ("Requested network type (%s) is not supported.", networkType.c_str()));
+                      &out,
+                      ("Requested network type (%s) is not supported.", network_type.c_str()));
     return;
   }
 
@@ -578,25 +599,27 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso
     LogAndReportError(common::PlatformResult(common::ErrorCode::NETWORK_ERR,
                                              "The networkType of the given DownloadRequest "
                                              "is currently not available on this device."),
-                      &out, ("Requested network type (%s) is not available.", networkType.c_str()));
+                      &out,
+                      ("Requested network type (%s) is not available.", network_type.c_str()));
     return;
   }
 
+  int ret;
   CallbackPtr downCbPtr(new DownloadCallback);
 
-  downCbPtr->callbackId = diPtr->callbackId;
+  downCbPtr->callbackId = di_ptr->callbackId;
   downCbPtr->instance = this;
 
   download_callbacks[downCbPtr->callbackId] = downCbPtr;
 
-  ret = download_create(&diPtr->download_id);
+  ret = download_create(&di_ptr->download_id);
   if (ret != DOWNLOAD_ERROR_NONE) {
     LogAndReportError(convertError(ret), &out,
                       ("download_create error: %d (%s)", ret, get_error_message(ret)));
     return;
   }
 
-  ret = download_set_state_changed_cb(diPtr->download_id, OnStateChanged,
+  ret = download_set_state_changed_cb(di_ptr->download_id, OnStateChanged,
                                       static_cast<void*>(downCbPtr));
   if (ret != DOWNLOAD_ERROR_NONE) {
     LogAndReportError(convertError(ret), &out, ("download_set_state_changed_cb error: %d (%s)", ret,
@@ -604,7 +627,7 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso
     return;
   }
 
-  ret = download_set_progress_cb(diPtr->download_id, progress_changed_cb,
+  ret = download_set_progress_cb(di_ptr->download_id, progress_changed_cb,
                                  static_cast<void*>(downCbPtr));
   if (ret != DOWNLOAD_ERROR_NONE) {
     LogAndReportError(convertError(ret), &out,
@@ -612,15 +635,15 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso
     return;
   }
 
-  ret = download_set_url(diPtr->download_id, diPtr->url.c_str());
+  ret = download_set_url(di_ptr->download_id, di_ptr->url.c_str());
   if (ret != DOWNLOAD_ERROR_NONE) {
     LogAndReportError(convertError(ret), &out,
                       ("download_set_url error: %d (%s)", ret, get_error_message(ret)));
     return;
   }
 
-  if (diPtr->destination.size() != 0) {
-    ret = download_set_destination(diPtr->download_id, diPtr->destination.c_str());
+  if (di_ptr->destination.size() != 0) {
+    ret = download_set_destination(di_ptr->download_id, di_ptr->destination.c_str());
     if (ret != DOWNLOAD_ERROR_NONE) {
       LogAndReportError(convertError(ret), &out,
                         ("download_set_destination error: %d (%s)", ret, get_error_message(ret)));
@@ -628,8 +651,8 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso
     }
   }
 
-  if (!diPtr->file_name.empty()) {
-    ret = download_set_file_name(diPtr->download_id, diPtr->file_name.c_str());
+  if (!di_ptr->file_name.empty()) {
+    ret = download_set_file_name(di_ptr->download_id, di_ptr->file_name.c_str());
     if (ret != DOWNLOAD_ERROR_NONE) {
       LogAndReportError(convertError(ret), &out,
                         ("download_set_file_name error: %d (%s)", ret, get_error_message(ret)));
@@ -637,19 +660,19 @@ void DownloadInstance::DownloadManagerStart(const picojson::value& args, picojso
     }
   }
 
-  ret = download_set_network_type(diPtr->download_id, diPtr->network_type);
+  ret = download_set_network_type(di_ptr->download_id, di_ptr->network_type);
 
   if (args.get("httpHeader").is<picojson::object>()) {
     picojson::object obj = args.get("httpHeader").get<picojson::object>();
     for (picojson::object::const_iterator it = obj.begin(); it != obj.end(); ++it) {
-      download_add_http_header_field(diPtr->download_id, it->first.c_str(),
+      download_add_http_header_field(di_ptr->download_id, it->first.c_str(),
                                      it->second.to_str().c_str());
     }
   }
 
-  diMap[downCbPtr->callbackId] = diPtr;
+  diMap[downCbPtr->callbackId] = di_ptr;
 
-  ret = download_start(diPtr->download_id);
+  ret = download_start(di_ptr->download_id);
 
   if (ret == DOWNLOAD_ERROR_NONE) {
     ReportSuccess(out);

diff --git a/src/download/download_instance.h b/src/download/download_instance.h
index f628736..82ee99b 100644 (file)
--- a/src/download/download_instance.h
+++ b/src/download/download_instance.h
@@ -45,29 +45,6 @@ class DownloadInstance : public common::ParsedInstance {
   virtual ~DownloadInstance();
 
  private:
-  void DownloadManagerStart(const picojson::value& args, picojson::object& out);
-  void DownloadManagerCancel(const picojson::value& args, picojson::object& out);
-  void DownloadManagerPause(const picojson::value& args, picojson::object& out);
-  void DownloadManagerResume(const picojson::value& args, picojson::object& out);
-  void DownloadManagerGetstate(const picojson::value& args, picojson::object& out);
-  void DownloadManagerGetdownloadrequest(const picojson::value& args, picojson::object& out);
-  void DownloadManagerGetmimetype(const picojson::value& args, picojson::object& out);
-  void DownloadManagerSetlistener(const picojson::value& args, picojson::object& out);
-
-  bool GetDownloadID(const int callback_id, int& download_id);
-
-  static common::PlatformResult convertError(int err, const std::string& message = "");
-  static void OnStateChanged(int download_id, download_state_e state, void* user_data);
-  static void progress_changed_cb(int download_id, long long unsigned received, void* user_data);
-  static void OnStart(int download_id, void* user_data);
-
-  static gboolean OnProgressChanged(void* user_data);
-  static gboolean OnFinished(void* user_data);
-  static gboolean OnPaused(void* user_data);
-  static gboolean OnCanceled(void* user_data);
-  static gboolean OnFailed(void* user_data);
-  static bool CheckInstance(DownloadInstance* instance);
-
   struct DownloadInfo {
     int callbackId;
     std::string url;
@@ -93,6 +70,32 @@ class DownloadInstance : public common::ParsedInstance {
   typedef std::shared_ptr<DownloadInfo> DownloadInfoPtr;
   typedef std::map<int, DownloadInfoPtr> DownloadInfoMap;
 
+  common::PlatformResult CheckNetworkConnection(const std::string& network_type,
+                                                bool& network_support, bool& network_available,
+                                                DownloadInfoPtr di_ptr);
+  void DownloadManagerStart(const picojson::value& args, picojson::object& out);
+  void DownloadManagerCancel(const picojson::value& args, picojson::object& out);
+  void DownloadManagerPause(const picojson::value& args, picojson::object& out);
+  void DownloadManagerResume(const picojson::value& args, picojson::object& out);
+  void DownloadManagerGetstate(const picojson::value& args, picojson::object& out);
+  void DownloadManagerGetdownloadrequest(const picojson::value& args, picojson::object& out);
+  void DownloadManagerGetmimetype(const picojson::value& args, picojson::object& out);
+  void DownloadManagerSetlistener(const picojson::value& args, picojson::object& out);
+
+  bool GetDownloadID(const int callback_id, int& download_id);
+
+  static common::PlatformResult convertError(int err, const std::string& message = "");
+  static void OnStateChanged(int download_id, download_state_e state, void* user_data);
+  static void progress_changed_cb(int download_id, long long unsigned received, void* user_data);
+  static void OnStart(int download_id, void* user_data);
+
+  static gboolean OnProgressChanged(void* user_data);
+  static gboolean OnFinished(void* user_data);
+  static gboolean OnPaused(void* user_data);
+  static gboolean OnCanceled(void* user_data);
+  static gboolean OnFailed(void* user_data);
+  static bool CheckInstance(DownloadInstance* instance);
+
   static std::mutex instances_mutex_;
   static std::vector<DownloadInstance*> instances_;