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