Re: [PATCH_v9] connman: add plugin in oFono to request/release private network

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3517 bytes --]

Hi Guillaume,

<snip>

> +static void request_reply(DBusPendingCall *call, void *user_data)
> +{
> +	struct pns_req *req = user_data;
> +	struct ofono_private_network_settings pns;
> +	DBusMessageIter array, dict, entry;
> +	DBusMessage *reply;
> +	const char *path;
> +
> +	DBG("");
> +
> +	pns.fd = -1;
> +	pns.server_ip = NULL;
> +	pns.peer_ip = NULL;
> +	pns.primary_dns = NULL;
> +	pns.secondary_dns = NULL;
> +
> +	req->pending = NULL;
> +
> +	reply = dbus_pending_call_steal_reply(call);
> +	if (!reply)
> +		goto error;
> +
> +	if (dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR)
> +		goto error;
> +
> +	if (dbus_message_iter_init(reply, &array) == FALSE)
> +		goto error;
> +
> +	if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_OBJECT_PATH)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&array, &path);
> +
> +	dbus_message_iter_next(&array);
> +	if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_ARRAY)
> +		goto error;
> +
> +	if (req->redundant == TRUE)
> +		goto release;
> +
> +	dbus_message_iter_recurse(&array, &dict);
> +
> +	while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) {
> +		DBusMessageIter iter;
> +		const char *key;
> +		int type;
> +
> +		dbus_message_iter_recurse(&dict, &entry);
> +
> +		dbus_message_iter_get_basic(&entry, &key);
> +
> +		dbus_message_iter_next(&entry);
> +		dbus_message_iter_recurse(&entry, &iter);
> +
> +		type = dbus_message_iter_get_arg_type(&iter);
> +		if (type != DBUS_TYPE_STRING)
> +			break;
> +
> +		if (g_str_equal(key, "ServerIPv4") &&
> +				type == DBUS_TYPE_STRING)
> +			dbus_message_iter_get_basic(&iter, &pns.server_ip);
> +		else if (g_str_equal(key, "PeerIPv4") &&
> +				type == DBUS_TYPE_STRING)
> +			dbus_message_iter_get_basic(&iter, &pns.peer_ip);
> +		else if (g_str_equal(key, "PrimaryDNS") &&
> +				type == DBUS_TYPE_STRING)
> +			dbus_message_iter_get_basic(&iter, &pns.primary_dns);
> +		else if (g_str_equal(key, "SecondaryDNS") &&
> +				type == DBUS_TYPE_STRING)
> +			dbus_message_iter_get_basic(&iter, &pns.secondary_dns);
> +
> +		dbus_message_iter_next(&dict);
> +	}
> +
> +	dbus_message_iter_next(&array);
> +	if (dbus_message_iter_get_arg_type(&array) != DBUS_TYPE_UNIX_FD)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&array, &pns.fd);
> +	DBG("Fildescriptor = %d\n", pns.fd);
> +
> +	req->path = g_strdup(path);
> +	DBG("Object path = %s\n", req->path);
> +
> +	if (pns.server_ip == NULL || pns.peer_ip == NULL ||
> +			pns.primary_dns == NULL || pns.secondary_dns == NULL ||
> +			pns.fd < 0) {
> +		ofono_error("Error while reading dictionnary...\n");
> +		goto release;
> +	}
> +
> +	req->cb(&pns, req->data);
> +
> +	dbus_message_unref(reply);
> +	dbus_pending_call_unref(call);
> +
> +	return;
> +
> +release:
> +	pns_release(req->uid);
> +error:
> +	if (pns.fd >= 0)
> +		close(pns.fd);
> +
> +	if (req->redundant == FALSE)
> +		req->cb(NULL, req->data);

And how do you think you will be using req->redundant when you just
freed req in pns_release above?

> +
> +	if (reply)
> +		dbus_message_unref(reply);
> +
> +	dbus_pending_call_unref(call);
> +}
> +

Anyhow, I applied this patch and refactored it very heavily afterward.
Please test my changes, particularly the error conditions with an older
version of ConnMan.

If things are working nicely please send a patch marking the DUN
emulator task as done.

Regards,
-Denis