Revise error handling in mm_sound_proxy.c

[Version] 0.12.23
[Issue Type] Refactor and bug fix

Change-Id: If0b5bfa27d2d434b5a126df6a9d7ecd66e06e691
Signed-off-by: Sangchul Lee <sc11.lee@samsung.com>

3 files changed, 106 insertions, 129 deletions

diff --git a/common/mm_sound_dbus.c b/common/mm_sound_dbus.c
index fcd43b3..6707dc9 100644
--- a/common/mm_sound_dbus.c
+++ b/common/mm_sound_dbus.c
@@ -212,6 +212,7 @@ static int _dbus_method_call(GDBusConnection *conn, const char *bus_name, const
 			debug_error("intf null");
 		if (!method)
 			debug_error("method null");
+		g_variant_unref(args);
 		return MM_ERROR_INVALID_ARGUMENT;
 	}
 
@@ -238,7 +239,8 @@ static int _dbus_method_call(GDBusConnection *conn, const char *bus_name, const
 		debug_log("Method Call '%s.%s' Success", intf, method);
 	}
 
-	*result = dbus_reply;
+	if (result)
+		*result = dbus_reply;
 
 	return ret;
 }
@@ -344,15 +346,18 @@ int mm_sound_dbus_method_call_to(audio_provider_t provider, audio_method_t metho
 
 	if (method_type >= AUDIO_METHOD_MAX) {
 		debug_error("Invalid method type : %d", method_type);
+		g_variant_unref(args);
 		return MM_ERROR_INVALID_ARGUMENT;
 	}
 	if (provider >= AUDIO_PROVIDER_MAX) {
 		debug_error("Invalid provider : %d", provider);
+		g_variant_unref(args);
 		return MM_ERROR_INVALID_ARGUMENT;
 	}
 
 	if (!(conn = _dbus_get_connection(G_BUS_TYPE_SYSTEM))) {
 		debug_error("Get Dbus Connection Error");
+		g_variant_unref(args);
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
diff --git a/mm_sound_proxy.c b/mm_sound_proxy.c
index 2a946c8..a1fb15b 100644
--- a/mm_sound_proxy.c
+++ b/mm_sound_proxy.c
@@ -292,29 +292,27 @@ int mm_sound_proxy_test(int a, int b, int *get)
 
 	debug_fenter();
 
-	params = g_variant_new("(ii)", a, b);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_TEST, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus test call failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(ii)", a, b)) == NULL) {
 		debug_error("Construct Param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_TEST, params, &result)) != MM_ERROR_NONE) {
+		debug_error("dbus test call failed");
+		g_variant_unref(result);
+		return ret;
+	}
+
 	if (result) {
 		g_variant_get(result, "(i)",  &reply);
 		debug_log("reply : %d", reply);
 		*get = reply;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
 }
@@ -322,7 +320,7 @@ cleanup:
 int mm_sound_proxy_is_stream_on_device(int stream_id, int device_id, bool *is_on)
 {
 	int ret = MM_ERROR_NONE;
-	GVariant *params, *result;
+	GVariant *params = NULL, *result = NULL;
 	gboolean _is_on;
 
 	debug_fenter();
@@ -352,10 +350,7 @@ int mm_sound_proxy_is_stream_on_device(int stream_id, int device_id, bool *is_on
 	}
 
 cleanup:
-	if (params)
-		g_variant_unref(params);
-	if (result)
-		g_variant_unref(result);
+	g_variant_unref(result);
 
 	debug_fleave();
 	return ret;
@@ -440,8 +435,7 @@ int mm_sound_proxy_get_current_connected_device_list(int device_flags, GList** d
 					device_item->name, device_item->vendor_id, device_item->product_id, device_item->is_running);
 			device_item->stream_num = -1;
 		} else {
-			if (device_item)
-				g_free(device_item);
+			g_free(device_item);
 			break;
 		}
 	}
@@ -458,8 +452,7 @@ int mm_sound_proxy_get_current_connected_device_list(int device_flags, GList** d
 #endif /* TIZEN_TV */
 
 cleanup:
-	if (result)
-		g_variant_unref(result);
+	g_variant_unref(result);
 
 	debug_fleave();
 	return ret;
@@ -515,8 +508,7 @@ int mm_sound_proxy_get_device_by_id(int device_id, mm_sound_device_t **device)
 	}
 
 cleanup:
-	if (result)
-		g_variant_unref(result);
+	g_variant_unref(result);
 
 	debug_fleave();
 	return ret;
@@ -664,30 +656,28 @@ int mm_sound_proxy_set_volume_by_type(const char *volume_type, const unsigned vo
 
 	debug_fenter();
 
-	params = g_variant_new("(ssu)", direction, volume_type, volume_level);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_SET_VOLUME_LEVEL, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus set volume by type failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(ssu)", direction, volume_type, volume_level)) == NULL) {
 		debug_error("Construct Param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_SET_VOLUME_LEVEL, params, &result)) != MM_ERROR_NONE) {
+		debug_error("dbus set volume by type failed");
+		g_variant_unref(result);
+		return ret;
+	}
+
 	if (result) {
 		g_variant_get(result, "(&s)",  &reply);
 		debug_log("reply : %s", reply);
 		if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
 			ret = MM_ERROR_SOUND_INTERNAL;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
 }
@@ -706,7 +696,6 @@ int mm_sound_proxy_add_volume_changed_callback(mm_sound_volume_changed_wrapper_c
 	else
 		*subs_id = cb_data->subs_id;
 
-
 	debug_fleave();
 
 	return ret;
@@ -732,30 +721,27 @@ int mm_sound_proxy_set_filter_by_type(const char *stream_type, const char *filte
 
 	debug_fenter();
 
-	params = g_variant_new("(ssss)", filter_name, filter_parameters, filter_group, stream_type);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_SET_FILTER, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus set filter by type failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(ssss)", filter_name, filter_parameters, filter_group, stream_type)) == NULL) {
 		debug_error("construct param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_SET_FILTER, params, &result)) != MM_ERROR_NONE)
+		debug_error("dbus set filter by type failed");
+
+	/* stream-manager always returns a string as return value */
 	if (result) {
 		g_variant_get(result, "(&s)", &reply);
 		debug_log("reply : %s", reply);
 		if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
 			ret = MM_ERROR_SOUND_INTERNAL;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		if (ret == MM_ERROR_NONE)
+			ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
 }
@@ -768,30 +754,27 @@ int mm_sound_proxy_unset_filter_by_type(const char *stream_type)
 
 	debug_fenter();
 
-	params = g_variant_new("(s)", stream_type);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_UNSET_FILTER, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus unset filter by type failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(s)", stream_type)) == NULL) {
 		debug_error("construct param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_UNSET_FILTER, params, &result)) != MM_ERROR_NONE)
+		debug_error("dbus unset filter by type failed");
+
+	/* stream-manager always returns a string as return value */
 	if (result) {
 		g_variant_get(result, "(&s)", &reply);
 		debug_log("reply : %s", reply);
 		if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
 			ret = MM_ERROR_SOUND_INTERNAL;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		if (ret == MM_ERROR_NONE)
+			ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
 }
@@ -804,29 +787,28 @@ int mm_sound_proxy_control_filter_by_type(const char *stream_type, const char *f
 
 	debug_fenter();
 
-	params = g_variant_new("(sss)", filter_name, filter_controls, stream_type);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_CONTROL_FILTER, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus control filter by type failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(sss)", filter_name, filter_controls, stream_type)) == NULL) {
 		debug_error("construct param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_CONTROL_FILTER, params, &result)) != MM_ERROR_NONE)
+		debug_error("dbus control filter by type failed");
+
+	/* stream-manager always returns a string as return value */
 	if (result) {
 		g_variant_get(result, "(&s)", &reply);
 		debug_log("reply : %s", reply);
 		if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
 			ret = MM_ERROR_SOUND_INTERNAL;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		if (ret == MM_ERROR_NONE)
+			ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
+	g_variant_unref(result);
 
 	debug_fleave();
 	return ret;
@@ -845,28 +827,27 @@ int mm_sound_proxy_play_tone_with_stream_info(int client_pid, int tone, char *st
 		return MM_ERROR_INVALID_ARGUMENT;
 	}
 
-	params = g_variant_new("(iiiisi)", tone, repeat, volume, client_pid, stream_type, stream_index);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_DTMF_WITH_STREAM_INFO, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus play tone failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(iiiisi)", tone, repeat, volume, client_pid, stream_type, stream_index)) == NULL) {
 		debug_error("Construct Param for method call failed");
+		return MM_ERROR_SOUND_INTERNAL;
+	}
+
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_DTMF_WITH_STREAM_INFO, params, &result)) != MM_ERROR_NONE) {
+		debug_error("dbus play tone failed");
+		g_variant_unref(result);
+		return ret;
 	}
 
 	if (result) {
 		g_variant_get(result, "(i)",  &handle);
 		debug_log("handle : %d", handle);
 		*codechandle = handle;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
 }
@@ -885,32 +866,29 @@ int mm_sound_proxy_play_sound_with_stream_info(const char* filename, int repeat,
 
 	debug_fenter();
 
-	params = g_variant_new("(siiisi)", filename, repeat, volume, client_pid, stream_type, stream_index);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_FILE_START_WITH_STREAM_INFO, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus play file failed");
-			goto cleanup;
-		}
-	} else {
+	if ((params = g_variant_new("(siiisi)", filename, repeat, volume, client_pid, stream_type, stream_index)) == NULL) {
 		debug_error("Construct Param for method call failed");
+		return MM_ERROR_SOUND_INTERNAL;
+	}
+
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_FILE_START_WITH_STREAM_INFO, params, &result)) != MM_ERROR_NONE) {
+		debug_error("dbus play file failed");
+		g_variant_unref(result);
+		return ret;
 	}
 
 	if (result) {
 		g_variant_get(result, "(i)",  &handle);
 		debug_log("handle : %d", handle);
 		*codechandle = handle;
+		g_variant_unref(result);
 	} else {
 		debug_error("reply null");
+		ret = MM_ERROR_SOUND_INTERNAL;
 	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
-
 	debug_fleave();
 	return ret;
-
-
 }
 
 int mm_sound_proxy_stop_sound(int handle)
@@ -920,14 +898,10 @@ int mm_sound_proxy_stop_sound(int handle)
 
 	debug_fenter();
 
-	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_FILE_STOP, g_variant_new("(i)", handle), &result)) != MM_ERROR_NONE) {
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_SOUND_SERVER, AUDIO_METHOD_PLAY_FILE_STOP, g_variant_new("(i)", handle), &result)) != MM_ERROR_NONE)
 		debug_error("dbus stop file playing failed");
-		goto cleanup;
-	}
 
-cleanup:
-	if (result)
-		g_variant_unref(result);
+	g_variant_unref(result);
 
 	debug_fleave();
 	return ret;
@@ -1053,21 +1027,19 @@ int mm_sound_proxy_acquire_focus(int index, mm_sound_focus_type_e type, int opti
 	} else {
 		GVariant *params = NULL, *result = NULL;
 
-		params = g_variant_new("(iiiis)", pid, id, type, option, ext_info ? ext_info : "");
-		if (params) {
-			if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_FOCUS_SERVER, AUDIO_METHOD_ACQUIRE_FOCUS, params, &result)) != MM_ERROR_NONE)
-				debug_error("dbus acquire focus failed");
-		} else {
+		if ((params = g_variant_new("(iiiis)", pid, id, type, option, ext_info ? ext_info : "")) == NULL) {
 			debug_error("Construct Param for method call failed");
+			return MM_ERROR_SOUND_INTERNAL;
 		}
 
-		if (ret != MM_ERROR_NONE)
-			g_variant_get(result, "(i)",  &ret);
-		if (result)
-			g_variant_unref(result);
+		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_FOCUS_SERVER, AUDIO_METHOD_ACQUIRE_FOCUS, params, &result)) != MM_ERROR_NONE)
+			debug_error("dbus acquire focus failed");
+
+		g_variant_unref(result);
 	}
 
 	debug_fleave();
+
 	return ret;
 }
 
@@ -1088,21 +1060,19 @@ int mm_sound_proxy_release_focus(int index, mm_sound_focus_type_e type, int opti
 	} else {
 		GVariant *params = NULL, *result = NULL;
 
-		params = g_variant_new("(iiiis)", pid, id, type, option, ext_info ? ext_info : "");
-		if (params) {
-			if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_FOCUS_SERVER, AUDIO_METHOD_RELEASE_FOCUS, params, &result)) != MM_ERROR_NONE)
-				debug_error("dbus release focus failed");
-		} else {
+		if ((params = g_variant_new("(iiiis)", pid, id, type, option, ext_info ? ext_info : "")) == NULL) {
 			debug_error("Construct Param for method call failed");
+			return MM_ERROR_SOUND_INTERNAL;
 		}
 
-		if (ret != MM_ERROR_NONE)
-			g_variant_get(result, "(i)",  &ret);
-		if (result)
-			g_variant_unref(result);
+		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_FOCUS_SERVER, AUDIO_METHOD_RELEASE_FOCUS, params, &result)) != MM_ERROR_NONE)
+			debug_error("dbus release focus failed");
+
+		g_variant_unref(result);
 	}
 
 	debug_fleave();
+
 	return ret;
 }
 
@@ -1114,26 +1084,28 @@ int mm_sound_proxy_update_stream_focus_status(int focus_id, unsigned int status)
 
 	debug_fenter();
 
-	params = g_variant_new("(iu)", focus_id, status);
-	if (params) {
-		if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_UPDATE_STREAM_FOCUS_STATUS, params, &result)) != MM_ERROR_NONE) {
-			debug_error("dbus set volume by type failed");
-			if (result) {
-				g_variant_get(result, "(&s)",  &reply);
-				debug_log("reply : %s", reply);
-				if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
-					ret = MM_ERROR_SOUND_INTERNAL;
-			}
-		}
-	} else {
+	if ((params = g_variant_new("(iu)", focus_id, status)) == NULL) {
 		debug_error("Construct Param for method call failed");
 		return MM_ERROR_SOUND_INTERNAL;
 	}
 
-	if (result)
+	if ((ret = mm_sound_dbus_method_call_to(AUDIO_PROVIDER_STREAM_MANAGER, AUDIO_METHOD_UPDATE_STREAM_FOCUS_STATUS, params, &result)) != MM_ERROR_NONE)
+		debug_error("dbus set volume by type failed");
+
+	/* stream-manager always returns a string as return value */
+	if (result) {
+		g_variant_get(result, "(&s)", &reply);
+		debug_log("reply : %s", reply);
+		if (strcmp(reply, "STREAM_MANAGER_RETURN_OK"))
+			ret = MM_ERROR_SOUND_INTERNAL;
 		g_variant_unref(result);
+	} else {
+		debug_error("reply null");
+		ret = MM_ERROR_SOUND_INTERNAL;
+	}
 
 	debug_fleave();
+
 	return ret;
 }
 
diff --git a/packaging/libmm-sound.spec b/packaging/libmm-sound.spec
index 8ecda7c..6b0af52 100644
--- a/packaging/libmm-sound.spec
+++ b/packaging/libmm-sound.spec
@@ -1,6 +1,6 @@
 Name:       libmm-sound
 Summary:    MMSound Package contains client lib and sound_server binary
-Version:    0.12.22
+Version:    0.12.23
 Release:    0
 Group:      System/Libraries
 License:    Apache-2.0