On Tue, 2009-06-30 at 05:44 -0700, Marcel Holtmann wrote:
> Hi Josh,
> 
> > > > When using the D-Bus API I've found that both Device and Manager have a
> > > > property each where when using SetProperty they return an invalid
> > > > arguments error when setting the property to the same value as is
> > > > already set.
> > > > 
> > > > This seems a bit draconian so I've attached a patch to return silently
> > > > instead.
> > > 
> > > Of course, it doesn't build and I didn't check before sending.
> > > What an idiot ...
> > 
> > Correct patch attached
> 
> it is still wrong. You can't just return NULL here. You need to return a
> proper success message.

OK, should have spent longer reading the code. Apologies for that.

What about something like this?

        if (g_str_equal(name, "Powered") == TRUE) {
                connman_bool_t powered;
                int err;

                if (type != DBUS_TYPE_BOOLEAN)
                        return __connman_error_invalid_arguments(msg);

                dbus_message_iter_get_basic(&value, &powered);

                if (device->pending != NULL)
                        return __connman_error_in_progress(msg);

                if (device->powered != powered) {
                        err = set_powered(device, powered);
                        if (err < 0 && err != -EINPROGRESS)
                                return __connman_error_failed(msg,
-err);
                }

                device->pending = dbus_message_ref(msg);

                device->timeout = g_timeout_add_seconds(15,
                                                powered_timeout, device);

                return NULL;

> 
> Also we are using tabs and you used something else. Coding style, coding
> style ;)

Sorry, still fighting my editor when using coding styles other than my
own.

> 
> Question still is if we really want this. Is this a desired behavior of
> the API. Does it makes things a lot simpler?

I think so. I report invalid argument errors back to the caller in
gconnman, invalid argument doesn't seem like the right error here.
I'm not even convinced this should be an error and at the very least it
quietens my debug output.

Regards,
Joshua
-- 
Joshua Lock
        Intel Open Source Technology Centre

_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman

Reply via email to