[systemd-devel] [RFC 5/6] proxy-discoveryd: Add the basic parts for handling DBus methods

[Lennart Poettering]

On Fri, 10.04.15 15:33, Marcel Holtmann (marcel at holtmann.org) wrote:

> Hi Lennart,
> 
> >> +
> >> +static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
> >> +        _cleanup_free_ char *p = strdup("DIRECT");
> > 
> > Please don't mix variable declarations and function invocations in one
> > line (also see CODING_STYLE). Also, missing OOM check...
> > 
> >> +        Manager *m = userdata;
> >> +        int r;
> >> +
> >> +        assert(bus);
> >> +        assert(message);
> >> +        assert(m);
> >> +
> >> +        r = proxy_execute(m->default_proxies, message);
> >> +        if (r < 0)
> >> +                sd_bus_reply_method_return(message, "s", p);
> > 
> > Hmm, is this right? Shouldn't we return the error code to the client
> > instead of eating up and returning "DIRECT"?
> > 
> > Also, why allocate "DIRECT" with strdup() at all?
> 
> There are no errors. Either you get a proxy directive or you return
> DIRECT to indicate no proxy. What would you do in an error case
> anyway. The backup is always assume no proxy.

Well, so far it has been our coding styles to propagate errors up if
they cause the invoked operations to fail, and allow the higher up
code deal with them and possibly eat them up. I mean, the HTTP client
can eat the error up and downgrade to DIRECT on its own, there's no
need to do this from our side already.

Lennart

-- 
Lennart Poettering, Red Hat