[PATCH v2 05/16] openconnect: Add asking cookie from agent

Patrik Flykt patrik.flykt at linux.intel.com
Thu Nov 29 02:49:33 PST 2012


	Hi,

On Tue, 2012-11-27 at 12:51 +0200, Jukka Rissanen wrote:
> +static connman_bool_t check_reply_has_dict(DBusMessage *reply)
> +{
> +	const char *signature = DBUS_TYPE_ARRAY_AS_STRING
> +		DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> +		DBUS_TYPE_STRING_AS_STRING
> +		DBUS_TYPE_VARIANT_AS_STRING
> +		DBUS_DICT_ENTRY_END_CHAR_AS_STRING;
> +
> +	if (dbus_message_has_signature(reply, signature) == TRUE)
> +		return TRUE;
> +
> +	connman_warn("Reply %s to %s from %s has wrong signature %s",
> +			signature,
> +			dbus_message_get_interface(reply),
> +			dbus_message_get_sender(reply),
> +			dbus_message_get_signature(reply));
> +
> +	return FALSE;
> +}
> +
> +static void request_input_append_name(DBusMessageIter *iter, void *user_data)
>  {
> -	const char *vpnhost, *vpncookie, *cafile, *certsha1, *mtu;
> -	int fd, err = 0;
> +	struct vpn_provider *provider = user_data;
> +	const char *str = "string";
> +
> +	connman_dbus_dict_append_basic(iter, "Type",
> +				DBUS_TYPE_STRING, &str);
> +	str = "informational";
> +	connman_dbus_dict_append_basic(iter, "Requirement",
> +				DBUS_TYPE_STRING, &str);
> +
> +	str = vpn_provider_get_name(provider);
> +	connman_dbus_dict_append_basic(iter, "Value",
> +				DBUS_TYPE_STRING, &str);
> +}
>  
> -	vpnhost = vpn_provider_get_string(provider, "Host");
> -	if (!vpnhost) {
> -		connman_error("Host not set; cannot enable VPN");
> -		err = -EINVAL;
> +static void request_input_append_host(DBusMessageIter *iter, void *user_data)
> +{
> +	struct vpn_provider *provider = user_data;
> +	const char *str = "string";
> +
> +	connman_dbus_dict_append_basic(iter, "Type",
> +				DBUS_TYPE_STRING, &str);
> +	str = "informational";
> +	connman_dbus_dict_append_basic(iter, "Requirement",
> +				DBUS_TYPE_STRING, &str);
> +
> +	str = vpn_provider_get_host(provider);
> +	connman_dbus_dict_append_basic(iter, "Value",
> +				DBUS_TYPE_STRING, &str);
> +}
> +

This part of the code repeats itself in openconnect.c, l2tp.c and
pptp.c. Please factor it out.

> +	connman_dbus_dict_append_dict(&dict, "OpenConnect.Cookie",
> +			request_input_append_cookie, provider);
> +
> +	connman_dbus_dict_append_dict(&dict, "Host",
> +			request_input_append_host, provider);
> +
> +	connman_dbus_dict_append_dict(&dict, "Name",
> +			request_input_append_name, provider);

Since these are going to be repeated in at least three plugins, please
consider if the request_input_append_* functions can do the
dict-append-dict part also. It is true it generates another set of
functions like request_input_append_*_attributes, but might easier to
keep in sync over multiple plugins.

Cheers,

	Patrik





More information about the connman mailing list