Fix API behavior to match the specification

- Issue:
Some APIs did not work as specified on API's documentation.

- Solution:
This patch adds logic for checking state and parameter and also fix some
function to work as specifedd on API's documentation. Through this
patch, the APIs will properly and expectedly work.

Change-Id: I8a4c0a36806ebfa562d18da13e3673bb776dd4da
Signed-off-by: Suyeon Hwang <stom.hwang@samsung.com>

diff --git a/client/vc.c b/client/vc.c
index 60b67b1570b85474fb65d1392f49328300d6edbb..fab3a6a8e42970e6467d835de3b681a8d48482f2 100644 (file)
--- a/client/vc.c
+++ b/client/vc.c
@@ -41,13 +41,12 @@
 #include "voice_control_command_expand.h"
 
 
-
-static Ecore_Timer* g_connect_timer = NULL;
-
 static Ecore_Event_Handler* g_focus_in_handler = NULL;
 static Ecore_Event_Handler* g_focus_out_handler = NULL;
 
 static Ecore_Thread* g_tts_thread = NULL;
+static Ecore_Thread* g_prepare_thread = NULL;
+static pthread_mutex_t g_prepare_thread_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static int g_pid;
 
@@ -446,11 +445,11 @@ int vc_deinitialize(void)
                __vc_internal_unprepare();
                /* no break. need to next step*/
        case VC_STATE_INITIALIZED:
-               if (NULL != g_connect_timer) {
-                       SLOG(LOG_DEBUG, TAG_VCC, "Connect Timer is deleted"); //LCOV_EXCL_LINE
-                       ecore_timer_del(g_connect_timer);
-                       g_connect_timer = NULL;
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (NULL != g_prepare_thread) {
+                       ecore_thread_cancel(g_prepare_thread);
                }
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
 
                vc_config_mgr_unset_lang_cb(g_pid);
                vc_config_mgr_finalize(g_pid);
@@ -495,8 +494,6 @@ static Eina_Bool __vc_connect_daemon(void *data)
        int mgr_pid = -1;
        int service_state = 0;
 
-       g_connect_timer = NULL;
-
        /* check handle */
        if (true == vc_client_is_valid()) {
                SLOG(LOG_DEBUG, TAG_VCC, "[DEBUG] The current client is valid");
@@ -617,12 +614,21 @@ static void __start_prepare_thread(void *data, Ecore_Thread *thread)
 
        /* Send hello */
        while (0 != ret) {
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (EINA_TRUE == ecore_thread_check(thread)) {
+                       SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
+                       return;
+               }
+
                if (retry_count == VC_TIDL_RETRY_COUNT) { // 200 ms * 20 = 4 sec
                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
                        return;
                }
 
                ret = vc_tidl_request_hello();
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
                if (ret == 0) {
                        SLOG(LOG_DEBUG, TAG_VCC, "Success to request hello. retry count(%d)", retry_count);
                        break;
@@ -635,23 +641,38 @@ static void __start_prepare_thread(void *data, Ecore_Thread *thread)
        ret = -1;
        retry_count = 0;
        while (0 != ret) {
+               pthread_mutex_lock(&g_prepare_thread_mutex);
+               if (EINA_TRUE == ecore_thread_check(thread)) {
+                       SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to request hello !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
+                       return;
+               }
+
                if (retry_count == 10) {
                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Fail to connect daemon !!"); //LCOV_EXCL_LINE
+                       pthread_mutex_unlock(&g_prepare_thread_mutex);
                        return;
                }
+
                ret = __vc_connect_daemon(NULL);
+               pthread_mutex_unlock(&g_prepare_thread_mutex);
                if (ret == 0)
                        break;
                else
                        retry_count++;
        }
-
-       return;
 }
 
 static void __end_prepare_thread(void *data, Ecore_Thread *thread)
 {
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ End prepare thread");
+       g_prepare_thread = NULL;
+}
+
+static void __cancel_prepare_thread(void *data, Ecore_Thread *thread)
+{
+       SLOG(LOG_DEBUG, TAG_VCC, "@@@ End prepare thread");
+       g_prepare_thread = NULL;
 }
 
 int vc_prepare(void)
@@ -672,8 +693,7 @@ int vc_prepare(void)
        /* check state */
        RETVM_IF(state != VC_STATE_INITIALIZED, VC_ERROR_INVALID_STATE, TAG_VCC, "[ERROR] Invalid State: Current state(%d) is not 'Initialized'", state);
 
-       ecore_thread_run(__start_prepare_thread, __end_prepare_thread, NULL, NULL);
-
+       g_prepare_thread = ecore_thread_run(__start_prepare_thread, __end_prepare_thread, __cancel_prepare_thread, NULL);
 
        return VC_ERROR_NONE;
 }
@@ -818,24 +838,21 @@ int vc_get_state(vc_state_e* state)
 
        vc_state_e temp;
        if (0 != vc_client_get_client_state(&temp)) {
-               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] handle is not valid");
-               return VC_ERROR_INVALID_STATE;
+               SLOG(LOG_WARN, TAG_VCC, "[ERROR] handle is not valid");
+               temp = VC_STATE_NONE;
        }
 
-
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ [Client] Get State");
-
-       *state = temp;
-
-       switch (*state) {
-       //LCOV_EXCL_START
+       switch (temp) {
        case VC_STATE_NONE:             SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'None'");            break;
        case VC_STATE_INITIALIZED:      SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'Created'");         break;
        case VC_STATE_READY:            SLOG(LOG_DEBUG, TAG_VCC, "Current state is 'Ready'");           break;
-       default:                        SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid state");
-       //LCOV_EXCL_STOP
+       default:
+               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid state. Return state as 'None'.");
+               temp = VC_STATE_NONE;
        }
 
+       *state = temp;
        SLOG(LOG_DEBUG, TAG_VCC, "@@@ [Client] Get State DONE");
 
        return VC_ERROR_NONE;
@@ -1195,6 +1212,12 @@ int vc_unset_command_list(int type)
        if (VC_ERROR_NONE != ret)
                return ret;
 
+       /* check type */
+       if ((VC_COMMAND_TYPE_FOREGROUND != type) && (VC_COMMAND_TYPE_BACKGROUND != type)) {
+               SLOG(LOG_ERROR, TAG_VCC, "[ERROR] Invalid command type: input type is %d", type);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_state_e state;
        if (0 != vc_client_get_client_state(&state)) {
                SLOG(LOG_ERROR, TAG_VCC, "[ERROR] A handle is not available");
@@ -1254,6 +1277,9 @@ int vc_set_command_list_from_file(const char* file_path, int type)
        if (VC_ERROR_NONE != ret)
                return ret;
 
+       RETVM_IF(NULL == file_path, VC_ERROR_INVALID_PARAMETER, TAG_VCM, "[ERROR] Invalid parameter");
+       SLOG(LOG_INFO, TAG_VCC, "@@@ File path: %s", file_path);
+
        vc_state_e state;
        if (0 != vc_client_get_client_state(&state)) {
                SLOG(LOG_ERROR, TAG_VCC, "[ERROR] A handle is not available");

diff --git a/common/vc_cmd_db.c b/common/vc_cmd_db.c
index 154a4fa77300f58f7b226d8cdf3508415cd659c4..2ddbb479ec542d62b8a116b30ad4699cacb9e4f5 100644 (file)
--- a/common/vc_cmd_db.c
+++ b/common/vc_cmd_db.c
@@ -2490,6 +2490,18 @@ int vc_db_delete_commands(int pid, vc_cmd_type_e type, char* appid)
        return VC_DB_ERROR_NONE;
 }
 
+int vc_db_delete_commands_without_reset_transaction(int pid, vc_cmd_type_e type, char* appid)
+{
+       int ret = 0;
+       ret = __vc_db_delete_commands(g_db_handle, pid, type, appid);
+       if (ret != VC_DB_ERROR_NONE) {
+               __vc_db_rollback_transaction(g_db_handle);
+               return ret;
+       }
+
+       return VC_DB_ERROR_NONE;
+}
+
 static void __free_command_list_element(gpointer data)
 {
        vc_cmd_h temp_cmd = (vc_cmd_h)data;

diff --git a/common/vc_cmd_db.h b/common/vc_cmd_db.h
index 87376e54f1df7e8757861ff39179dd1821a502a5..040f2af51ac0ee26fbcd681e69b7414a698727b3 100644 (file)
--- a/common/vc_cmd_db.h
+++ b/common/vc_cmd_db.h
@@ -86,6 +86,8 @@ int vc_db_select_by_pid(int pid);
 
 int vc_db_delete_commands(int pid, vc_cmd_type_e type, char* appid);
 
+int vc_db_delete_commands_without_reset_transaction(int pid, vc_cmd_type_e type, char* appid);
+
 int vc_db_backup_command();
 
 int vc_db_restore_command();

diff --git a/common/vc_command.c b/common/vc_command.c
index 77e801643be77c6db472183fb8ff3171e0d00950..1c63b22deeea0fd10effc94855a4c95ffda0fcee 100644 (file)
--- a/common/vc_command.c
+++ b/common/vc_command.c
@@ -356,16 +356,13 @@ int vc_cmd_list_remove(vc_cmd_list_h vc_cmd_list, vc_cmd_h vc_command)
        return VC_ERROR_NONE;
 }
 
-static void __vc_cmd_list_remove_all_foreach(gpointer data, gpointer user_data)
+static void __vc_cmd_list_remove_all_foreach(gpointer data)
 {
-       vc_cmd_s *temp = NULL;
-       temp = data;
+       vc_cmd_s *temp = (vc_cmd_s *)data;
        if (temp) {
-               SLOG(LOG_DEBUG, TAG_VCCMD, "Free command(%p)", temp);
+               SECURE_SLOG(LOG_DEBUG, TAG_VCCMD, "Free command(%p)", temp);
                vc_cmd_destroy((vc_cmd_h)temp);
-               temp = NULL;
        }
-       return;
 }
 
 int vc_cmd_list_remove_all(vc_cmd_list_h vc_cmd_list, bool release_command)
@@ -384,21 +381,25 @@ int vc_cmd_list_remove_all(vc_cmd_list_h vc_cmd_list, bool release_command)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
-       vc_cmd_list_s* list = NULL;
-       list = (vc_cmd_list_s*)vc_cmd_list;
+       vc_cmd_list_s* cmd_list = (vc_cmd_list_s*)vc_cmd_list;
 
        SLOG(LOG_DEBUG, TAG_VCCMD, "[List] list (%p), release command (%s)"
-                , list, release_command ? "true" : "false");
+                , cmd_list, release_command ? "true" : "false");
+
+       if (NULL == cmd_list->list) {
+               SLOG(LOG_DEBUG, TAG_VCCMD, "@@@ List is already empty.");
+               return VC_ERROR_NONE;
+       }
 
        if (true == release_command) {
-               if (list->list) {
-                       g_slist_foreach(list->list, __vc_cmd_list_remove_all_foreach, NULL);
-                       g_slist_free(list->list);
-                       list->list = NULL;
-               }
-               list->index = -1;
+               g_slist_free_full(cmd_list->list, __vc_cmd_list_remove_all_foreach);
+       } else {
+               g_slist_free(cmd_list->list);
        }
 
+       cmd_list->list = NULL;
+       cmd_list->index = -1;
+
        SLOG(LOG_DEBUG, TAG_VCCMD, "@@@");
 
        return VC_ERROR_NONE;
@@ -413,7 +414,7 @@ int vc_cmd_list_foreach_commands(vc_cmd_list_h vc_cmd_list, vc_cmd_list_cb callb
                return VC_ERROR_PERMISSION_DENIED;
        }
 
-       if (NULL == vc_cmd_list) {
+       if (NULL == vc_cmd_list || NULL == callback) {
                SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Input parameter is NULL");
                return VC_ERROR_INVALID_PARAMETER;
        }
@@ -452,7 +453,7 @@ int vc_cmd_list_filter_by_type(vc_cmd_list_h original, int type, vc_cmd_list_h*
                return VC_ERROR_PERMISSION_DENIED;
        }
 
-       if (NULL == original) {
+       if (NULL == original || NULL == filtered) {
                SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Input parameter is NULL");
                return VC_ERROR_INVALID_PARAMETER;
        }
@@ -598,6 +599,10 @@ int vc_cmd_list_next(vc_cmd_list_h vc_cmd_list)
        list = (vc_cmd_list_s*)vc_cmd_list;
 
        int count = g_slist_length(list->list);
+       if (0 == count) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] List is empty");
+               return VC_ERROR_EMPTY;
+       }
 
        if (list->index < count - 1) {
                list->index = list->index + 1;
@@ -627,6 +632,11 @@ int vc_cmd_list_prev(vc_cmd_list_h vc_cmd_list)
        vc_cmd_list_s* list = NULL;
        list = (vc_cmd_list_s*)vc_cmd_list;
 
+       if (0 == g_slist_length(list->list)) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] List is empty");
+               return VC_ERROR_EMPTY;
+       }
+
        if (list->index > 0) {
                list->index = list->index - 1;
                SLOG(LOG_DEBUG, TAG_VCCMD, "[DEBUG] List index : %d", list->index);
@@ -1107,6 +1117,11 @@ int vc_cmd_set_type(vc_cmd_h vc_command, int type)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (VC_COMMAND_TYPE_NONE >= type || VC_COMMAND_TYPE_EXCLUSIVE < type) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid type(%d)", type);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 
@@ -1152,6 +1167,11 @@ int vc_cmd_set_format(vc_cmd_h vc_command, int format)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (VC_COMMAND_FORMAT_FIXED > format || VC_COMMAND_FORMAT_NONFIXED_AND_FIXED < format) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid format(%d)", format);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 
@@ -1197,6 +1217,11 @@ int vc_cmd_set_pid(vc_cmd_h vc_command, int pid)
                return VC_ERROR_INVALID_PARAMETER;
        }
 
+       if (0 > pid) {
+               SLOG(LOG_ERROR, TAG_VCCMD, "[ERROR] Invalid pid (%d)", pid);
+               return VC_ERROR_INVALID_PARAMETER;
+       }
+
        vc_cmd_s* cmd = NULL;
        cmd = (vc_cmd_s*)vc_command;
 

diff --git a/common/vc_json_parser.c b/common/vc_json_parser.c
index 63e838b79d9370cb83aa8ba2765bfb2a62201f77..3fc4eea56acfc9841106e13b065a46acd71f72ef 100644 (file)
--- a/common/vc_json_parser.c
+++ b/common/vc_json_parser.c
@@ -282,7 +282,7 @@ static int __vc_json_set_commands(JsonObject *root_obj, int type, char* invocati
 
                        if (0 != strncmp(cmd->appid, prev_appid, strlen(cmd->appid))) {
                                //delete background commands with appid and type
-                               ret = vc_db_delete_commands(cmd->pid, type, cmd->appid);
+                               ret = vc_db_delete_commands_without_reset_transaction(cmd->pid, type, cmd->appid);
                                if (VC_ERROR_NONE != ret) {
                                        SLOG(LOG_ERROR, vc_json_tag(), "[ERROR] Fail to insert command into db, ret(%d) pid(%d) type(%d), appid(%s)", ret, cmd->pid, type, cmd->appid);
                                        free(temp_type);