hi Patrik,
On Wed, Mar 21, 2012 at 2:17 PM, Patrik Flykt
<[email protected]>wrote:
>
> Hi,
>
> On Wed, 2012-03-21 at 13:08 +0200, Alok Barsode wrote:
> > + if (service->timeservers_config != NULL) {
> > + guint len = g_strv_length(service->timeservers_config);
> > +
> > + g_key_file_set_string_list(keyfile, service->identifier,
> > +
> "Timeservers",
> > + (const gchar **)
> service->timeservers_config, len);
> > + } else
> > + g_key_file_remove_key(keyfile, service->identifier,
> > + "Timeservers",
> NULL);
>
> Indentation with that else.
>
> > +char **connman_service_get_timeservers_config(struct connman_service
> *service)
> > +{
> > + if (service == NULL)
> > + return NULL;
> > +
> > + if (service->timeservers_config != NULL)
> > + return g_strdupv(service->timeservers_config);
>
> Why can't we just return service->timeservers_config here and let the
> user strdup it if needed?
>
what if the programmer forgets to strdupv it and tries to access it when
the service is freed? :P
>
> > char **connman_service_get_timeservers(struct connman_service *service)
> > {
> > if (service == NULL)
> > @@ -2704,6 +2733,41 @@ static DBusMessage *set_property(DBusConnection
> *conn,
> > dns_configuration_changed(service);
> >
> > service_save(service);
> > + } else if (g_str_equal(name, "Timeservers.Configuration") == TRUE)
> {
> > + DBusMessageIter entry;
> > + GString *str;
>
> Instead of a GString and reallocating strings all the time...
>
> > + dbus_message_iter_recurse(&value, &entry);
> > +
> > + while (dbus_message_iter_get_arg_type(&entry) ==
> DBUS_TYPE_STRING) {
> > + const char *val;
> > + dbus_message_iter_get_basic(&entry, &val);
> > + dbus_message_iter_next(&entry);
> > + if (str->len > 0)
> > + g_string_append_printf(str, " %s", val);
> > + else
> > + g_string_append(str, val);
> > + }
>
> ...I'd create a GSList with the g_strdup'ed values and in the end shovel
> those into the newly allocated service->timeservers_config.
>
sure.
>
>
> Cheers,
>
> Patrik
>
> Cheers,
Alok.
>
> _______________________________________________
> connman mailing list
> [email protected]
> http://lists.connman.net/listinfo/connman
>
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
