Hi Daniel,

On Thu, Oct 21, 2010 at 04:35:34PM -0400, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wag...@bmw-carit.de>
Looks pretty good, I just have a few comments:

> +struct vpn_data {
> +     struct connman_provider *provider;
> +     char *if_name;
> +     unsigned flags;
> +     unsigned int watch;
> +     unsigned int state;
> +     struct connman_task *task;
> +};
> +
> +struct vpn_driver_data {
> +     const char *name;
> +     struct vpn_driver *vpn_driver;
> +     struct connman_provider_driver provider_driver;
> +};
> +
> +GSList *vpn_list = NULL;
I think it would make sense to have a hash table for the vpn driver list, or
at least an alphabetically sorted list. With an hash table, you could get rid
of the function below:

> +static struct vpn_driver_data *find_vpn_driver_data_by_name(const char *name)
> +{
> +     GSList *list;
> +
> +     for (list = vpn_list; list; list = list->next) {
> +             struct vpn_driver_data *data = list->data;
> +
> +             if (strcmp(data->name, name) == 0)
> +                     return data;
> +     }
> +
> +     return NULL;
> +}




> +
> +struct vpn_driver {
> +     int (*notify) (DBusMessage *msg, struct connman_provider *provider);
> +     int (*connect) (struct connman_provider *provider,
> +                     struct connman_task *task, int *stdin_fd,
> +                     const char *if_name);
> +     void (*disconnect) (void);
We probably don't need the disconnect hook at the moment.

Everything else looks fine. Please note that I haven't had time to actually
test it agaisnt our cisco VPN servers. I'll do that later today.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman

Reply via email to