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

[Marcel Holtmann]

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.

of course it can, but it also does not help you at all. You are not making anything better here.

I think that one clean interface to this needs to be a API compatible libproxy. Similar to what we did in PACrunner since that allows existing application to just use systemd-proxydiscoverd.

Regards

Marcel