From: Jung Jihoon Date: Tue, 9 Jul 2019 12:24:30 +0000 (+0900) Subject: Fix Coverity issues X-Git-Tag: submit/tizen/20190710.084405~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c1a56c53bb0ac2bcf46054efac1f9d28de8efdf0;p=platform%2Fcore%2Fapi%2Fmulti-device-group.git Fix Coverity issues - Logically dead code: 1051469 - Dereference null return value : 1051467 - Unchecked return value : 1051464 - resource leak: 1051461 1043634 1043622 1043615 1043603 1043597 - no effect: 1043629 1043627 1043625 1043618 - Uninitialized scalar variable: 1043605 1051462 Change-Id: I558827611b721d92fbcbbc2811b5b9501a41ceed Signed-off-by: Jung Jihoon --- diff --git a/src/mdg.c b/src/mdg.c index 301d52f..52e8997 100755 --- a/src/mdg.c +++ b/src/mdg.c @@ -94,25 +94,6 @@ EXPORT_API int mdg_group_create(mdg_h handle, char *group_name) return ret; } -EXPORT_API void mdg_group_destroy(mdg_group_s *group) -{ - CHECK_FEATURE_SUPPORTED(MDG_FEATURE); - - mdg_check_null_ret("group", group); - - if (group->device_id) { - g_free(group->device_id); - group->device_id = NULL; - } - if (group->group_name) { - g_free(group->group_name); - group->group_name = NULL; - } - - g_free(group); - group = NULL; -} - EXPORT_API int mdg_group_find(mdg_h handle, int timeout, mdg_group_found_cb found_cb, mdg_group_find_finish_cb finish_cb, void *user_data) @@ -428,7 +409,7 @@ EXPORT_API int mdg_channel_read(mdg_channel_h channel, unsigned char **data, int mdg_check_null_ret_error("channel", channel, MDG_ERROR_INVALID_PARAMETER); *len = read(_channel->client_sockfd, *data, MAXBUF); - if (ret == -1) { + if (*len == -1) { char buf[128]; strerror_r(errno, buf, 128); _ERR("read() error : %s", buf); @@ -728,20 +709,9 @@ EXPORT_API int mdg_group_info_destroy(mdg_group_h data) CHECK_FEATURE_SUPPORTED(MDG_FEATURE); - mdg_group_s *group = (mdg_group_s *)data; - mdg_check_null_ret_error("group", group, MDG_ERROR_INVALID_PARAMETER); - - if (group->device_id) { - g_free(group->device_id); - group->device_id = NULL; - } - if (group->group_name) { - g_free(group->group_name); - group->group_name = NULL; - } + mdg_check_null_ret_error("data", data, MDG_ERROR_INVALID_PARAMETER); - g_free(group); - group = NULL; + mdg_clean_group((mdg_group_s *)data); return ret; } @@ -832,41 +802,9 @@ EXPORT_API int mdg_device_info_destroy(mdg_device_h data) CHECK_FEATURE_SUPPORTED(MDG_FEATURE); - mdg_device_s * device = (mdg_device_s *)data; mdg_check_null_ret_error("data", data, MDG_ERROR_INVALID_PARAMETER); - if (device->device_id) { - g_free(device->device_id); - device->device_id = NULL; - } - - if (device->model_name) { - g_free(device->model_name); - device->model_name = NULL; - } - - if (device->device_name) { - g_free(device->device_name); - device->device_name = NULL; - } - - if (device->platform_ver) { - g_free(device->platform_ver); - device->platform_ver = NULL; - } - - if (device->vendor_id) { - g_free(device->vendor_id); - device->vendor_id = NULL; - } - - if (device->profile) { - g_free(device->profile); - device->profile = NULL; - } - - g_free(device); - device = NULL; + mdg_clean_device((mdg_device_s *)data); return ret; } diff --git a/src/mdg_dbus.c b/src/mdg_dbus.c old mode 100644 new mode 100755 index 5e439f0..6b1ef91 --- a/src/mdg_dbus.c +++ b/src/mdg_dbus.c @@ -50,10 +50,13 @@ static void __event_cb(Group *object, case MDG_EVENT_GROUP_FOUND: { mdg_group_s *group = NULL; - group = mdg_get_group_from_variant(va); if (handle->group_found_cb.found_cb) { + group = mdg_get_group_from_variant(va); + handle->group_found_cb.found_cb(group->type, group, handle->group_found_cb.user_data); + + mdg_clean_group(group); } else { _ERR("The callback not exists"); } @@ -71,10 +74,13 @@ static void __event_cb(Group *object, case MDG_EVENT_DEVICE_FOUND: { mdg_device_s *device = NULL; - device = mdg_get_device_from_variant(va); if (handle->device_found_cb.found_cb) { + device = mdg_get_device_from_variant(va); + handle->device_found_cb.found_cb(device, handle->device_found_cb.user_data); + + mdg_clean_device(device); } else { _ERR("The callback not exists"); } @@ -97,6 +103,8 @@ static void __event_cb(Group *object, } handle->device_invite_finish_cb.finish_cb(ret, device, handle->device_invite_finish_cb.user_data); + + mdg_clean_device(device); } else { _ERR("The callback not exists"); } @@ -118,11 +126,15 @@ static void __event_cb(Group *object, unsigned char *data = NULL; int data_len = 0; - mdg_get_data_from_variant(va, &device_id, &channel_id, &msg_id, &data, &data_len); - mdg_device_s *device = (mdg_device_s *)(handle->send_data_finish_cb.device); if (handle->send_data_finish_cb.finish_cb) { + mdg_device_s *device = (mdg_device_s *)(handle->send_data_finish_cb.device); + mdg_get_data_from_variant(va, &device_id, &channel_id, &msg_id, &data, &data_len); + handle->send_data_finish_cb.finish_cb(ret, device, channel_id, msg_id, data, data_len, handle->send_data_finish_cb.user_data); + mdg_clean_device(device); + if (data != NULL) + free(data); } else { _ERR("The callback not exists"); } @@ -171,6 +183,12 @@ static void __event_cb(Group *object, sscanf(channel->remote_address, "coaps://%s", (char *)tempaddr); ret_ptr = strtok_r(tempaddr, ":", &temp_for_strtok); + if (ret_ptr == NULL) { + _ERR("strtok failed"); + ret = MDG_ERROR_OPERATION_FAILED; + goto REQUEST_OPEN_CHANNEL_FINISH_EXIT; + } + _ERR("Address is %s", ret_ptr); inet_pton(AF_INET, ret_ptr, &serveraddr.sin_addr.s_addr); @@ -191,6 +209,9 @@ REQUEST_OPEN_CHANNEL_FINISH_EXIT: } else { _ERR("The callback not exists"); } + + mdg_clean_channel(channel); + break; } case MDG_EVENT_RECEIVE_DATA: { @@ -198,7 +219,7 @@ REQUEST_OPEN_CHANNEL_FINISH_EXIT: char *channel_id; int msg_id; unsigned char *data; - int data_len; + int data_len = 0; mdg_get_data_from_variant(va, &device_id, &channel_id, &msg_id, &data, &data_len); @@ -213,15 +234,21 @@ REQUEST_OPEN_CHANNEL_FINISH_EXIT: if (channel != NULL) channel->receive_data_cb(0, device_id, channel_id, msg_id, data, data_len, channel->receive_data_cb_user_data); + + if (data != NULL) + free(data); + break; } case MDG_EVENT_INVITED: { mdg_group_s *group = NULL; - group = mdg_get_group_from_variant(va); - if (handle->invited_event_cb.invited_event_cb) { + group = mdg_get_group_from_variant(va); + handle->invited_event_cb.invited_event_cb(group, handle->invited_event_cb.user_data); + + mdg_clean_group(group); } else { _ERR("The callback not exists"); } @@ -230,10 +257,12 @@ REQUEST_OPEN_CHANNEL_FINISH_EXIT: case MDG_EVENT_EJECTED: { mdg_group_s *group = NULL; - group = mdg_get_group_from_variant(va); - if (handle->ejected_event_cb.ejected_event_cb) { + group = mdg_get_group_from_variant(va); + handle->ejected_event_cb.ejected_event_cb(group, handle->ejected_event_cb.user_data); + + mdg_clean_group(group); } else { _ERR("The callback not exists"); } @@ -285,12 +314,24 @@ REQUEST_OPEN_CHANNEL_FINISH_EXIT: goto RECEIVE_OPEN_CHANNEL_EXIT; } - setsockopt(channel->server_sockfd, SOL_SOCKET, SO_RCVTIMEO, &tv_timeo, sizeof(tv_timeo)); - setsockopt(channel->server_sockfd, SOL_SOCKET, SO_LINGER, &ling, sizeof(ling)); + if (setsockopt(channel->server_sockfd, SOL_SOCKET, SO_RCVTIMEO, &tv_timeo, sizeof(tv_timeo)) < 0) { + ret = MDG_ERROR_OPERATION_FAILED; + strerror_r(errno, buf, 128); + _ERR("setsockopt error : %s", buf); + goto RECEIVE_OPEN_CHANNEL_EXIT; + } + + if (setsockopt(channel->server_sockfd, SOL_SOCKET, SO_LINGER, &ling, sizeof(ling)) < 0) { + ret = MDG_ERROR_OPERATION_FAILED; + strerror_r(errno, buf, 128); + _ERR("setsockopt error : %s", buf); + goto RECEIVE_OPEN_CHANNEL_EXIT; + } serveraddr.sin_family = AF_INET; serveraddr.sin_addr.s_addr = htonl(INADDR_ANY); serveraddr.sin_port = htons(8675); + memset(&serveraddr.sin_zero, 0, sizeof(serveraddr.sin_zero)); if (bind(channel->server_sockfd, (struct sockaddr *)&serveraddr, sizeof(serveraddr)) < 0) { ret = MDG_ERROR_OPERATION_FAILED; @@ -324,6 +365,9 @@ RECEIVE_OPEN_CHANNEL_EXIT: } else { _ERR("The callback not exists"); } + + mdg_clean_channel(channel); + break; } default: @@ -385,9 +429,9 @@ static void _dbus_name_owner_notify(GObject *object, GParamSpec *pspec, gpointer *user_data) { GDBusProxy *proxy = G_DBUS_PROXY(object); - gchar *name_owner = g_dbus_proxy_get_name_owner(proxy); - mdg_manager_s *handle = (mdg_manager_s *)user_data; mdg_check_null_ret("user_data", user_data); + mdg_manager_s *handle = (mdg_manager_s *)user_data; + gchar *name_owner = g_dbus_proxy_get_name_owner(proxy); LOGD("Name owner notify [%s]", name_owner); diff --git a/src/mdg_util.c b/src/mdg_util.c index 37b0f13..900ef14 100755 --- a/src/mdg_util.c +++ b/src/mdg_util.c @@ -240,3 +240,85 @@ GVariant *mdg_create_variant_device(mdg_device_s *device) return va; } + +void mdg_clean_group (mdg_group_s *group) +{ + if (group == NULL) + return; + + if (group->device_id) { + g_free(group->device_id); + group->device_id = NULL; + } + if (group->group_name) { + g_free(group->group_name); + group->group_name = NULL; + } + + g_free(group); + group = NULL; +} + +void mdg_clean_device (mdg_device_s *device) +{ + if (device == NULL) + return; + + if (device->device_id) { + g_free(device->device_id); + device->device_id = NULL; + } + + if (device->model_name) { + g_free(device->model_name); + device->model_name = NULL; + } + + if (device->device_name) { + g_free(device->device_name); + device->device_name = NULL; + } + + if (device->platform_ver) { + g_free(device->platform_ver); + device->platform_ver = NULL; + } + + if (device->vendor_id) { + g_free(device->vendor_id); + device->vendor_id = NULL; + } + + if (device->profile) { + g_free(device->profile); + device->profile = NULL; + } + + g_free(device); + device = NULL; +} + +void mdg_clean_channel (mdg_channel_s *channel) +{ + if (channel == NULL) + return; + + if (channel->device_id) { + g_free(channel->device_id); + channel->device_id = NULL; + } + + if (channel->channel_id) { + g_free(channel->channel_id); + channel->channel_id = NULL; + } + + if (channel->remote_address) { + g_free(channel->remote_address); + channel->remote_address = NULL; + } + + g_free(channel); + channel = NULL; +} + diff --git a/src/mdg_util.h b/src/mdg_util.h index 47619d3..3efe801 100755 --- a/src/mdg_util.h +++ b/src/mdg_util.h @@ -34,6 +34,9 @@ void mdg_get_data_from_variant(GVariant *va, char **device_id, char **channel_id void mdg_get_receive_file_from_variant(GVariant *va, char **device_id, char **file_path); void mdg_get_progress_from_variant(GVariant *va, char **file_path, long long *send_size, long long *total_size, int *percent); +void mdg_clean_group (mdg_group_s *group); +void mdg_clean_device (mdg_device_s *device); +void mdg_clean_channel (mdg_channel_s *channel); #ifdef __cplusplus } diff --git a/test/mdg-manager.c b/test/mdg-manager.c old mode 100644 new mode 100755 index 35fca8f..9babdee --- a/test/mdg-manager.c +++ b/test/mdg-manager.c @@ -538,6 +538,8 @@ bool _device_found_cb(mdg_device_h device, void *user_data) if (device_id) free(device_id); + if (model_name) + free(model_name); return TRUE; } @@ -738,6 +740,9 @@ bool _group_found_cb(mdg_group_type_e type, mdg_group_h group, void *user_data) found_group_list = g_list_append(found_group_list, group); + if (group_name != NULL) + free(group_name); + return TRUE; } @@ -960,7 +965,7 @@ static int run_request_create_group(MManager *mm, struct menu_data *menu) } int dev_idx = 1; - if (device_idx != NULL && strlen(device_idx)) { + if (strlen(device_idx)) { dev_idx = (unsigned short)strtol(device_idx, NULL, 10); if (0 >= dev_idx) { msgp("Invalid index. set to 1"); @@ -1007,7 +1012,7 @@ static int run_request_invite(MManager *mm, struct menu_data *menu) } int grp_idx = 1; - if (group_idx != NULL && strlen(group_idx)) { + if (strlen(group_idx)) { grp_idx = (unsigned short)strtol(group_idx, NULL, 10); if (0 >= grp_idx) { msgp("Invalid index. set to 1"); @@ -1024,7 +1029,7 @@ static int run_request_invite(MManager *mm, struct menu_data *menu) } int dev_idx = 1; - if (device_idx != NULL && strlen(device_idx)) { + if (strlen(device_idx)) { dev_idx = (unsigned short)strtol(device_idx, NULL, 10); if (0 >= dev_idx) { msgp("Invalid index. set to 1"); @@ -1142,7 +1147,7 @@ static int run_request_channel_list(MManager *mm, struct menu_data *menu) } int dev_idx = 1; - if (device_idx != NULL && strlen(device_idx)) { + if (strlen(device_idx)) { dev_idx = (unsigned short)strtol(device_idx, NULL, 10); if (0 >= dev_idx) { msgp("Invalid index. set to 1");