[PATCH] Allow vpn plugin to report error to user.

Samuel Ortiz sameo at linux.intel.com
Thu Jan 27 01:38:42 PST 2011


Hi Mohamed,

On Thu, Jan 27, 2011 at 12:03:56AM -0800, Mohamed Abbas wrote:
> If vpm client failed for auth or login problem, allow vpn plugin
> to report this error to user.
> ---
> David Woodhouse asked for this feature. I added the code to allow
> that once this accepted we need to add get_reason function for
> each vpn plugin to chech the exist code and return the right
> provider error code for that error.
The approach seems fine, I still have some comments though: 


> diff --git a/include/task.h b/include/task.h
> index f608c7e..bde2a29 100644
> --- a/include/task.h
> +++ b/include/task.h
> @@ -37,7 +37,7 @@ extern "C" {
>  struct connman_task;
>  
>  typedef void (* connman_task_exit_t) (struct connman_task *task,
> -							void *user_data);
> +						int exit_code, void *user_data);
>  
>  typedef void (* connman_task_notify_t) (struct connman_task *task,
>  				DBusMessage *message, void *user_data);
Please submit the task API improvement as a separate patch.


> diff --git a/plugins/vpn.c b/plugins/vpn.c
> index 278e0ea..ada2b82 100644
> --- a/plugins/vpn.c
> +++ b/plugins/vpn.c
> @@ -102,11 +102,12 @@ static int kill_tun(char *tun_name)
>  	return 0;
>  }
>  
> -void vpn_died(struct connman_task *task, void *user_data)
> +void vpn_died(struct connman_task *task, int exist_code, void *user_data)
Typo: exit_code, not exist_code.


>  {
>  	struct connman_provider *provider = user_data;
>  	struct vpn_data *data = connman_provider_get_data(provider);
>  	int state = data->state;
> +	enum connman_provider_error ret;
>  
>  	DBG("provider %p data %p", provider, data);
>  
> @@ -118,10 +119,21 @@ void vpn_died(struct connman_task *task, void *user_data)
>  	connman_rtnl_remove_watch(data->watch);
>  
>  vpn_exit:
> -	if (state != VPN_STATE_READY && state != VPN_STATE_DISCONNECT)
> -		connman_provider_set_state(provider,
> -						CONNMAN_PROVIDER_STATE_FAILURE);
> -	else
> +	if (state != VPN_STATE_READY && state != VPN_STATE_DISCONNECT) {
> +		const char *name;
> +		struct vpn_driver_data *vpn_driver_data;
> +
> +		name = connman_provider_get_driver_name(provider);
> +		vpn_driver_data = g_hash_table_lookup(driver_hash, name);
> +		if (vpn_driver_data != NULL &&
> +				vpn_driver_data->vpn_driver->get_reason != NULL)
> +			ret = vpn_driver_data->vpn_driver->get_reason(provider,
> +								exist_code);
> +		else
> +			ret = CONNMAN_PROVIDER_STATE_UNKNOWN;
CONNMAN_PROVIDER_ERROR_UNKNOWN


> +		connman_provider_indicate_error(provider, ret);
> +	} else
>  		connman_provider_set_state(provider,
>  						CONNMAN_PROVIDER_STATE_IDLE);
>  
> diff --git a/plugins/vpn.h b/plugins/vpn.h
> index 7f10150..b1945f3 100644
> --- a/plugins/vpn.h
> +++ b/plugins/vpn.h
> @@ -33,6 +33,7 @@ struct vpn_driver {
>  	int (*connect) (struct connman_provider *provider,
>  			struct connman_task *task, const char *if_name);
>  	void (*disconnect) (void);
> +	int (*get_reason) (struct connman_provider *provider, int exist_code);
Please rename it to error_code(), and s/exist/exit/. Also, not sure you need to
pass the provider pointer as an argument.

Also, it would be nice to provide the plugins implementation for the
error_code in the same patch set.


> diff --git a/src/task.c b/src/task.c
> index 541106c..3552261 100644
> --- a/src/task.c
> +++ b/src/task.c
> @@ -246,11 +246,15 @@ int connman_task_set_notify(struct connman_task *task, const char *member,
>  static void task_died(GPid pid, gint status, gpointer user_data)
>  {
>  	struct connman_task *task = user_data;
> +	int exist_code;
>  
> -	if (WIFEXITED(status))
> -		DBG("task %p exit status %d", task, WEXITSTATUS(status));
> -	else
> +	if (WIFEXITED(status)) {
> +		exist_code = WEXITSTATUS(status);
> +		DBG("task %p exit status %d", task, exist_code);
exist typo here as well.


> +	} else {
> +		exist_code = 0;
>  		DBG("task %p signal %d", task, WTERMSIG(status));
> +	}
>  
>  	g_spawn_close_pid(pid);
>  	task->pid = -1;
> @@ -258,7 +262,7 @@ static void task_died(GPid pid, gint status, gpointer user_data)
>  	task->child_watch = 0;
>  
>  	if (task->exit_func)
> -		task->exit_func(task, task->exit_data);
> +		task->exit_func(task, exist_code, task->exit_data);
Ditto.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/



More information about the connman mailing list