mot-agent: fixed crash due to unhandled response callback at CA layer.

OCMultipleOwnershipTransfer(), OCProvisionPairwiseDevices() and OCDelete
DeviceUUID() and etc use a 'wait time'. But if API caller calls OCCleanupForTimeout
before wait time, the callback function called from CA already became null.
There is a problem that the thread does not die.

Signed-off-by: saerome.kim <saerome.kim@samsung.com>

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b00eab1..0e9d0b6 100644 (file)
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -29,7 +29,7 @@ FOREACH(flag ${daemon_pkgs_CFLAGS})
 ENDFOREACH(flag)
 
 SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS} -fpic -std=gnu99")
-SET(CMAKE_CXX_FLAGS "${EXTRA_CXXFLAGS} -std=gnu++11 -fPIC -fvisibility=hidden")
+SET(CMAKE_CXX_FLAGS "${EXTRA_CXXFLAGS} -std=gnu++11 -fPIC -Wall -Werror-implicit-function-declaration -fvisibility=hidden")
 SET(CMAKE_EXE_LINKER_FLAGS "-Wl,--as-needed -pie")
 
 MESSAGE(" - Define...iotivity macro")

diff --git a/src/mdg-manager/src/mdgd_gdbus_group.c b/src/mdg-manager/src/mdgd_gdbus_group.c
index b4c6e53..af2576d 100644 (file)
--- a/src/mdg-manager/src/mdgd_gdbus_group.c
+++ b/src/mdg-manager/src/mdgd_gdbus_group.c
@@ -186,11 +186,7 @@ gboolean group_eject(Group *group, GDBusMethodInvocation *invocation, gchar *gro
        mdgd_context_t *mdgd_ctx = mdgd_context_get_context();
        mdgd_check_null_ret_error("mdgd_ctx", mdgd_ctx, FALSE);
 
-       if (!mdgd_check_device_exist(uuid)) {
-               result = MDGD_ERROR_ALREADY_REGISTERED;
-       } else {
-               result = mdgd_group_eject(group_name, mdgd_ctx->device_uuid, uuid);
-       }
+       result = mdgd_group_eject(group_name, mdgd_ctx->device_uuid, uuid);
 
        /* Now, for the sake of convenience, we change 'group_complete_eject' to 'group_complete_device_eject'. */
 #if 0

diff --git a/src/mdg-manager/src/mdgd_group.c b/src/mdg-manager/src/mdgd_group.c
index ffe7bcd..b86f7fa 100644 (file)
--- a/src/mdg-manager/src/mdgd_group.c
+++ b/src/mdg-manager/src/mdgd_group.c
@@ -141,13 +141,6 @@ int mdgd_group_create(const char* name)
        return MDGD_ERROR_NONE;
 }
 
-static void _free_device_func(gpointer data)
-{
-       mdgd_check_null_ret("data", data);
-       g_free(data);
-       data = NULL;
-}
-
 static void _free_group_func(gpointer data)
 {
        mdgd_group_t *group = (mdgd_group_t *)data;

diff --git a/src/mdg-manager/src/mdgd_iot.cpp b/src/mdg-manager/src/mdgd_iot.cpp
old mode 100755 (executable)
new mode 100644 (file)
index 50da357..2abf585
--- a/src/mdg-manager/src/mdgd_iot.cpp
+++ b/src/mdg-manager/src/mdgd_iot.cpp
@@ -402,7 +402,6 @@ OCEntityHandlerResult _request_handler(std::shared_ptr<OCResourceRequest> reques
                        char *model_name = NULL;
                        char *platform_ver = NULL;
                        char *profile = NULL;
-                       char *vendor_id = NULL;
 
                        system_settings_get_value_string(SYSTEM_SETTINGS_KEY_DEVICE_NAME, &device_name);
                        system_info_get_platform_string(SYSTEM_INFO_MODEL_NAME, &model_name);
@@ -697,7 +696,6 @@ static void _on_post(const HeaderOptions& /*headerOptions*/,
                                         const OCRepresentation& rep, const int eCode,
                                         void *user_data)
 {
-       int ret;
        mdgd_command_t *cmd = (mdgd_command_t *)user_data;
 
        try {
@@ -727,15 +725,11 @@ static void _on_get(const HeaderOptions& /*headerOptions*/,
                                const OCRepresentation& rep, const int eCode,
                                void *user_data)
 {
-       int ret;
        LOG_DEBUG("On get function.");
 
        try {
                if (eCode == OC_STACK_OK || eCode == OC_STACK_RESOURCE_CREATED ||
                        eCode == OC_STACK_RESOURCE_CHANGED) {
-                       gchar *key;
-                       GVariant *val;
-                       gsize len = 0;
                        mdgd_mot_device_t *device = (mdgd_mot_device_t *)user_data;
                        mdgd_check_null_ret("device", device);
 
@@ -781,7 +775,7 @@ static int _perform_mot_me(gpointer data)
        mdgd_context_t *mdgd_ctx = mdgd_context_get_context();
        mdgd_check_null_ret_error("mdgd_ctx", mdgd_ctx, FALSE);
 
-       ret = agent_mot(mdgd_ctx->device_uuid, "12341234");
+       ret = agent_mot(mdgd_ctx->device_uuid, (char *)"12341234");
        if (ret != MDGD_ERROR_NONE)
                LOG_ERR("agent_mot(%s) Failed", mdgd_ctx->device_uuid);
 
@@ -791,7 +785,6 @@ static int _perform_mot_me(gpointer data)
 static bool _found_resource(std::shared_ptr<OCResource> resource,
                                                        void *user_data)
 {
-       int ret;
        char *resource_uri_path;
        char *resource_device_id = NULL;
        char *resource_host;
@@ -847,8 +840,7 @@ static bool _found_resource(std::shared_ptr<OCResource> resource,
                                GVariant *device_info;
                                bool mowned;
                                GVariantIter *iter = NULL;
-                               gchar *key;
-                               mdgd_mot_device_t *device = NULL;
+                               mdgd_mot_device_t *device = NULL;
 
                                if (NULL == parameters) {
                                        LOG_ERR("No Multiple Owned devices found");
@@ -960,7 +952,6 @@ static bool _found_resource(std::shared_ptr<OCResource> resource,
                                        char *model_name = NULL;
                                        char *platform_ver = NULL;
                                        char *profile = NULL;
-                                       char *vendor_id = NULL;
 
                                        system_settings_get_value_string(SYSTEM_SETTINGS_KEY_DEVICE_NAME, &device_name);
                                        system_info_get_platform_string(SYSTEM_INFO_MODEL_NAME, &model_name);

diff --git a/src/mot-agent/ma-log.h b/src/mot-agent/ma-log.h
index ec20a24..d0988e1 100644 (file)
--- a/src/mot-agent/ma-log.h
+++ b/src/mot-agent/ma-log.h
@@ -22,34 +22,31 @@
 #undef LOG_TAG
 #endif
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 #ifdef USE_DLOG
-#include <dlog.h>
+#include <dlog/dlog.h>
 
+#ifdef LOG_TAG
 #undef LOG_TAG
+#endif
 #define LOG_TAG "MOT_AGENT"
 
-#define COLOR_RED "\033[0;31m"
-#define COLOR_GREEN "\033[0;32m"
-#define COLOR_BROWN "\033[0;33m"
-#define COLOR_BLUE "\033[0;34m"
-#define COLOR_PURPLE "\033[0;35m"
-#define COLOR_CYAN "\033[0;36m"
-#define COLOR_LIGHTBLUE "\033[0;37m"
-#define COLOR_END "\033[0;m"
-
-#define MA_LOGV(format, args...) LOGV(format, ##args)
-#define MA_LOGD(format, args...) LOGD(COLOR_LIGHTBLUE" "format COLOR_END, ##args)
-#define MA_LOGI(format, args...) LOGI(COLOR_GREEN" "format COLOR_END, ##args)
-#define MA_LOGW(format, args...) LOGW(COLOR_CYAN" "format COLOR_END, ##args)
-#define MA_LOGE(format, args...) LOGE(COLOR_RED" "format COLOR_END, ##args)
-#define MA_LOGF(format, args...) LOGF(format, ##args)
+#define MA_LOGV(format, arg...) LOGD(format, ##arg)
+#define MA_LOGD(format, arg...) LOGD(format, ##arg)
+#define MA_LOGI(format, arg...) LOGI(format, ##arg)
+#define MA_LOGW(format, arg...) LOGW(format, ##arg)
+#define MA_LOGE(format, arg...) LOGE(format, ##arg)
+#define MA_LOGF(format, arg...) LOGE(format, ##arg)
 
 #define __MA_LOG_FUNC_ENTER__ LOGD("Enter")
-#define __MA_LOG_FUNC_EXIT__ LOGD("Quit")
+#define __MA_LOG_FUNC_EXIT__  LOGD("Quit")
 
 #define MA_SECLOGI(format, args...) SECURE_LOG(LOG_INFO, MA_LOG_TAG, format, ##args)
 #define MA_SECLOGD(format, args...) SECURE_LOG(LOG_DEBUG, MA_LOG_TAG, format, ##args)
-
 #else /* USE_DLOG */
 
 #define MA_LOGV(format, args...)
@@ -67,4 +64,8 @@
 
 #endif /* USE_DLOG */
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif /* __MA_LOG_H__ */

diff --git a/src/mot-agent/ma-subowner.c b/src/mot-agent/ma-subowner.c
index ca7e89b..2ef0167 100644 (file)
--- a/src/mot-agent/ma-subowner.c
+++ b/src/mot-agent/ma-subowner.c
@@ -18,6 +18,7 @@
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/time.h>
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
 #endif
@@ -45,8 +46,8 @@
 #define ACL_RESRC_ARRAY_SIZE 3 /**< This value is used only for sample (not OCF spec) */
 
 #define DISCOVERY_TIMEOUT  6 /**< 6 sec */
-#define CALLBACK_TIMEOUT_10S 10 * 1000 /**< 10 sec = 10 * 1000 * 1ms */
 #define CALLBACK_TIMEOUT_5S 5 * 1000 /**< 5sec = 5 * 1000 * 1ms */
+#define TIME_UNIT 100 /**< Sleep time unit */
 
 /* '_' for separaing from the same constant variable in srmresourcestrings.c  */
 static const char* SVR_DB_FILE_NAME = "oic_svr_db_ma.dat";
@@ -138,41 +139,45 @@ typedef struct _proc {
        pthread_mutex_t lock; /**< POSIX Mutex */
        pthread_cond_t cond; /**< POSIX condition */
        long int waits; /**< How long we have to wait */
-} PROC;
+} process_t;
 
-void _exec_and_wait(void* args)
+static void* _exec_and_wait(void* args)
 {
-       OCStackResult ret = OC_STACK_OK;
+       int ret = OC_STACK_OK;
+       process_t *proc = (process_t *)args;
        static struct timespec time_to_wait = {0, 0};
-       PROC *proc = (PROC*) args;
-       int TIMES = proc->waits /100;
 
-       time_to_wait.tv_sec = time(NULL) + proc->waits / 1000;
-       time_to_wait.tv_nsec = (proc->waits % 1000) * 1000000;
+       int TIMES = proc->waits / TIME_UNIT;
+       if (0 != proc->waits % TIME_UNIT)
+               TIMES += 1;
 
-       MA_LOGE("before increase proc[%s] data to [%d]", proc->name, proc->data);
+       while (!g_client->g_doneCB && proc->data <= TIMES) {
+               /* Increase 100 msec */
+               time_to_wait.tv_sec = time(NULL);
+               time_to_wait.tv_nsec = TIME_UNIT * 1000000;
 
-       while (g_client->g_doneCB && proc->data < TIMES) {
-               MA_LOGE("increase proc[%s] data to [%d]", proc->name, proc->data);
                pthread_mutex_lock(&proc->lock);
                pthread_cond_timedwait(&proc->cond, &proc->lock, &time_to_wait);
-               proc->data++;
+               pthread_mutex_unlock(&proc->lock);
 
+               g_mutex_lock(&g_client->iotivity_mutex);
                ret = OCProcess();
+               g_mutex_unlock(&g_client->iotivity_mutex);
+
                if (OC_STACK_OK != ret) {
                        MA_LOGE("OCStack process error = %d", ret);
-                       proc->data = TIMES;
+                       break;
                }
-
-               pthread_mutex_unlock(&proc->lock);
+               proc->data++;
        }
+       return NULL;
 }
 
-int _init_proc(PROC *proc, void *(*routine) (void *))
+static int _init_proc(process_t *proc, void *(*routine) (void *))
 {
        pthread_attr_t attr;
 
-       MA_LOGI("try to init proc [%s]", proc->name);
+       MA_LOGD("try to init proc [%s]", proc->name);
        pthread_attr_init(&attr);
        pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
 
@@ -194,7 +199,7 @@ static FILE* _fopen_prvn_mng(const char* path, const char* mode)
                        "/opt/usr/data", SVR_DB_FILE_NAME);
 
        NOTUSED(path);
-#if 0
+#ifdef DEBUG_SVR_DB
        MA_LOGD("Unsed DB path  %s\n", path);
 #endif
 
@@ -203,28 +208,31 @@ static FILE* _fopen_prvn_mng(const char* path, const char* mode)
 
 static int _wait_cb_ret(int msec)
 {
+       int ret = OC_STACK_OK;
+#ifndef THREAD_COND_WAIT_USED
        int result = -1;
-       OCStackResult ret = OC_STACK_OK;
        struct timespec tim, tim2;
        tim.tv_sec = 0;
        tim.tv_nsec = 100 * 1000000;
-
-       ma_check_null_ret_error("g_client", g_client, OC_STACK_INVALID_PARAM);
+#endif
 
 #ifdef THREAD_COND_WAIT_USED
-       int result_a = 0;
-       PROC proc_a;
+       void *result;
+       process_t proc;
 
-       strcpy(proc_a.name, "A\0");
-       proc_a.data = 0;
-       proc_a.waits = msec;
+       ma_check_null_ret_error("g_client", g_client, OC_STACK_INVALID_PARAM);
 
-       if (MA_ERROR_NONE != _init_proc(&proc_a, _exec_and_wait))
-               MA_LOGE("Fail to init proc [%s]", proc_a.name);
+       strcpy(proc.name, "A\0");
+       proc.data = 0;
+       proc.waits = msec + CALLBACK_TIMEOUT_5S;
 
-       pthread_join(proc_a.thread, (void*) &result_a);
+       if (0 != _init_proc(&proc, _exec_and_wait)) {
+               MA_LOGE("Fail to init proc [%s]", proc.name);
+               return OC_STACK_ERROR;
+       }
+       pthread_join(proc.thread, &result);
 #else
-       for (int i = 0; !g_client->g_doneCB && msec > i; i += 100) {
+       for (int i = 0; !g_client->g_doneCB && (msec + CALLBACK_TIMEOUT_5S) >= i; i += 10) {
                /*
                  * Basically, nanosleep is more thread-safe function,
                  * But, in our case, nanosleep cause crash frequently
@@ -243,8 +251,10 @@ static int _wait_cb_ret(int msec)
                }
        }
 #endif
-       if (!g_client->g_doneCB)
+       if (!g_client->g_doneCB) {
                OCPDMCleanupForTimeout();
+               ret = OC_STACK_TIMEOUT;
+       }
 
        return ret;
 }
@@ -1863,7 +1873,6 @@ static void _make_devices_unpair_func(ma_req_cb_s *con)
                if (OC_STACK_OK != ret)  {
                        MA_LOGD("OCUnlinkDevices Timeout = %d (%s)",
                                ret, ma_ocf_error_to_string(ret));
-                       goto PV_UNPAIR_END;
                }
        }
 
@@ -1876,7 +1885,6 @@ static void _make_devices_unpair_func(ma_req_cb_s *con)
        if (OC_STACK_OK  != ret) {
                MA_LOGE("GetDoxmDevOwnerId faild = [%d][%s]",
                        ret, ma_ocf_error_to_string(ret));
-               goto PV_UNPAIR_END;
        }
 
        g_mutex_lock(&g_client->iotivity_mutex);
@@ -1885,14 +1893,12 @@ static void _make_devices_unpair_func(ma_req_cb_s *con)
        if (OC_STACK_OK != ret) {
                MA_LOGE("OCRemoveSubOwner: ret = %d (%s)",
                        ret, ma_ocf_error_to_string(ret));
-               goto PV_UNPAIR_END;
        }
 
        ret = _wait_cb_ret(CALLBACK_TIMEOUT_5S);
        if (OC_STACK_OK  != ret) {
                MA_LOGE("OCRemoveSubOwner Timeout = %d (%s)",
                        ret, ma_ocf_error_to_string(ret));
-               goto PV_UNPAIR_END;
        }
 
        MA_LOGI("Remove Multiple Ownership Done");
@@ -1908,7 +1914,6 @@ static void _make_devices_unpair_func(ma_req_cb_s *con)
        if (OC_STACK_OK != ret) {
                MA_LOGD("OCRemoveDeviceWithUuid API error = %d (%s)",
                        ret, ma_ocf_error_to_string(ret));
-               goto PV_UNPAIR_END;
        }
 
        ret = _wait_cb_ret(CALLBACK_TIMEOUT_5S);

diff --git a/src/mot-agent/ma-subowner.h b/src/mot-agent/ma-subowner.h
index e341507..e67310f 100644 (file)
--- a/src/mot-agent/ma-subowner.h
+++ b/src/mot-agent/ma-subowner.h
@@ -37,6 +37,7 @@ typedef enum {
        MA_MAKE_PAIR, /**< Pair 2 devices */
        MA_MAKE_UNPAIR, /**< Unpair 2 devices */
        MA_DO_RESOURCES_PAIRWISE, /** < Pairwise Provisioning */
+       MA_CMD_MAX /**< Max command id */
 } ma_cmd_id_e;
 
 /* ACL Permission type */

diff --git a/src/mot-agent/ma-util.c b/src/mot-agent/ma-util.c
index b41ed9d..b9bfc8d 100644 (file)
--- a/src/mot-agent/ma-util.c
+++ b/src/mot-agent/ma-util.c
@@ -31,7 +31,7 @@
 #include "ma-util.h"
 
 #define CASE_TO_STR(x) case x: return #x;
-const char* ma_ocf_error_to_string(OCStackResult err)
+const char* ma_ocf_error_to_string(int err)
 {
        switch (err) {
        /* CHECK: List all enum values here */
@@ -83,6 +83,7 @@ const char* ma_ocf_error_to_string(OCStackResult err)
        CASE_TO_STR(OC_STACK_NOT_ACCEPTABLE)
        CASE_TO_STR(OC_STACK_METHOD_NOT_ALLOWED)
        CASE_TO_STR(OC_STACK_FORBIDDEN_REQ)
+       CASE_TO_STR(OC_STACK_TOO_MANY_REQUESTS)
        CASE_TO_STR(OC_STACK_INTERNAL_SERVER_ERROR)
        CASE_TO_STR(OC_STACK_NOT_IMPLEMENTED)
        CASE_TO_STR(OC_STACK_BAD_GATEWAY)
@@ -186,7 +187,7 @@ OicUuid_t* ma_convert_uuid(gchar *device_id)
 }
 
 char g_uuid_str[256] = {0};
-char * ma_get_readable_uuid(const OicUuid_t* uuid)
+char* ma_get_readable_uuid(const OicUuid_t* uuid)
 {
        memset(g_uuid_str, 0, sizeof(g_uuid_str));
        snprintf(g_uuid_str, sizeof(g_uuid_str),
@@ -198,25 +199,6 @@ char * ma_get_readable_uuid(const OicUuid_t* uuid)
        return g_uuid_str;
 }
 
-#ifdef TEST
-OCProvisionDev_t* ma_get_dev_by_id(const OCProvisionDev_t* dev_lst,
-       const int dev_num)
-{
-       if (!dev_lst || 0 >= dev_num) {
-               MA_LOGI("Device List is Empty..");
-               return NULL;
-       }
-
-       OCProvisionDev_t* lst = (OCProvisionDev_t*) dev_lst;
-       for (int i = 0; lst; ) {
-               if (dev_num == ++i)
-                       return lst;
-               lst = lst->next;
-       }
-       return NULL;
-}
-#endif
-
 OCProvisionDev_t* ma_get_dev_by_uuid(const OCProvisionDev_t* dev_lst,
        const OicUuid_t* uuid)
 {

diff --git a/src/mot-agent/ma-util.h b/src/mot-agent/ma-util.h
index 2ec8b81..9b5b9ef 100644 (file)
--- a/src/mot-agent/ma-util.h
+++ b/src/mot-agent/ma-util.h
@@ -42,6 +42,13 @@ extern "C"
 
 #define NOTUSED(var) (var = var)
 
+#define ma_check_null_print_ret_error(msg, value, error) do { \
+               if (G_UNLIKELY(NULL == (value))) { \
+                                       MA_LOGE("%s", msg); \
+                                       return error; \
+                               } \
+} while (FALSE)
+
 #define ma_check_null_ret_error(name, value, error) do { \
                if (G_UNLIKELY(NULL == (value))) { \
                                        MA_LOGE("%s is NULL", name); \
@@ -56,23 +63,16 @@ extern "C"
                                } \
 } while (FALSE)
 
-const char* ma_ocf_error_to_string(OCStackResult err);
+const char* ma_ocf_error_to_string(int err);
 const char* ma_wifi_error_to_string(wifi_manager_error_e err);
 const char * ma_erro_to_string(ma_error_e err);
 OicUuid_t* ma_convert_uuid(gchar *device_id);
 char * ma_get_readable_uuid(const OicUuid_t* uuid);
-#ifdef TEST
-OCProvisionDev_t* ma_get_dev_by_id(const OCProvisionDev_t* dev_lst, const int dev_num);
-#endif
 OCProvisionDev_t* ma_get_dev_by_uuid(const OCProvisionDev_t* dev_lst,
        const OicUuid_t* uuid);
 int ma_print_dev_list(const OCProvisionDev_t* dev_lst);
 int ma_print_result_list(const OCProvisionResult_t* rslt_lst, const int rslt_cnt);
 int ma_set_device_id_seed(void);
-
-#if 0
-static OCProvisionDev_t* _clone_ocprovision_dev(const OCProvisionDev_t* src);
-#endif
 int ConvertUuidToStr(const OicUuid_t* uuid, char** strUuid);
 
 #ifdef __cplusplus

diff --git a/src/mot-agent/ma.h b/src/mot-agent/ma.h
index 9ab7227..fcb7e5b 100644 (file)
--- a/src/mot-agent/ma.h
+++ b/src/mot-agent/ma.h
@@ -21,6 +21,11 @@
 #include <gio/gio.h>
 #include <tizen.h>
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 #ifndef TIZEN_ERROR_MDG
 #define TIZEN_ERROR_MDG -0x02F50000 /**< Base error code */
 #endif
@@ -58,5 +63,9 @@ typedef struct _ma_service {
        gboolean ma_activated; /**< Whether d2d-manager enabled or not */
 } ma_service;
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif /* __MA_AGENT_H__ */