Hi Samuel,
On Thu, 2011-03-10 at 01:22 +0100, Samuel Ortiz wrote:
> Hi Henri,
>
> On Thu, Feb 24, 2011 at 04:03:19PM +0200, Henri Bragge wrote:
> > +static DBusMessage *provision_service(DBusConnection *conn, DBusMessage
> > *msg,
> > + void *data)
> > +{
> > + int err;
> > +
> > + DBG("conn %p", conn);
> > +
> > + err = __connman_service_provision(msg);
> > + if (err < 0) {
> > + if (err == -EINPROGRESS) {
> > + connman_error("Invalid return code from connect");
> > + err = -EINVAL;
> > + }
> How can __connman_service_provision() return -EINPROGRESS ?
Fixed in v2.
>
> > +int __connman_service_provision(DBusMessage *msg)
> > +{
> > + GKeyFile *keyfile = NULL;
> > + const char *config_str = NULL;
> > + char *group = NULL, *ident = NULL;
> > + int err = 0;
> > +
> > + DBG("");
> > +
> > + dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &config_str,
> > + DBUS_TYPE_INVALID);
> > +
> > + if (config_str == NULL)
> > + return -EINVAL;
> > +
> > + keyfile = g_key_file_new();
> > +
> > + /* populate GKeyFile with config_str */
> > + if (g_key_file_load_from_data(keyfile, config_str,
> > + strlen(config_str), 0, NULL) == FALSE) {
> > + err = -EINVAL;
> > + goto done;
> > + }
> > +
> > + /*
> > + * read only one group of settings (only one service supported, no
> > + * global settings)
> > + */
> > + group = g_key_file_get_start_group(keyfile);
> Could we verify that the group string follows the "service_*" pattern ?
>
Fixed in v2.
For more robust handling of malformed user input, I also added a check
for empty config_str and handled the return value of
__connman_config_load_service() call.
>
> > + __connman_config_load_service(keyfile, group);
> > +
> > + ident = g_strdup(group + strlen("service_"));
> Why are you duplicating that string ? Wouldn't ident = group + strlen(); be
> enough ?
Fixed in v2 also, which I sent as a reply. Thanks for very good remarks.
- Henri
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
