[systemd-devel] [PATCH 2/3] sd-daemon: Use LISTEN_NAMES env when available

[Krzysztof Opasiak]

On 05/15/2015 05:40 PM, Lennart Poettering wrote:
> On Fri, 15.05.15 17:09, Krzysztof Opasiak (k.opasiak at samsung.com) wrote:
>
>> LISTEN_NAMES environment variable contains details
>> about received file descriptors. Let's try to use it
>> instead of doing always two stats.
>
> I am really not convinced that it is a good idea to store redundant
> information in LISTEN_NAMES, especially if we don't have this
> information in all cases anyway.

We also don't always have informations about paths as all descriptors 
from fd store have unknown path and type (as far as I know and 
understand systemd code but I may be wrong)

>
> Please, let's keep this simple: LISTEN_NAMES should only carry actual
> names, nothing else, and let's query the kernel for the actual fd
> types.

I'm not sure if doing stat() to determine how he should interpret 
content of field in env is simpler and easier but of course you are the 
maintainer so you decide how things should be done;)

>
> There's really no point in storing the types in $LISTEN_NAMEs, since
> this code is no way performance senstive...
>

Matching between fds and list of expected paths is done in n^2 so it has 
no performance impact as long as number of passed fds isn't big. I know 
that it is usually done only once during service startup but it increase 
latency between event occurrence and it processing.

>> +static const char *sd_get_fd_name(int fd) {
>
> The "sd_" prefix we add for exported functions, don't bother with it
> for internal calls.

Ok. Will fix this.

>
>> +        static const char sep = ':';
>> +        static const char escape = '\\';
>> +        const char *env = NULL;
>> +        const char *e = NULL;
>> +        int i;
>>
>> -        assert_return(fd >= 0, -EINVAL);
>> +        assert_return(fd >= 3, NULL);
>
> assert_return() we use for verifiying parameters passed in from
> external users to check for programming errors. Since this function is
> static this generally doesn't apply. See CODING_STYLE for details...
>

will replace with traditional if.

-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics