[systemd-devel] [PATCH] sysctl: don't replace dots with slashes in prefix

[Jan Synacek]

David Herrmann <dh.herrmann at gmail.com> writes:
> Hi
>
> On Fri, Sep 12, 2014 at 11:21 AM, Jan Synacek <jsynacek at redhat.com> wrote:
>> The prefix is always tested against normalized property names.
>> ---
>>  src/sysctl/sysctl.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c
>> index 8ce9870..0cb0875 100644
>> --- a/src/sysctl/sysctl.c
>> +++ b/src/sysctl/sysctl.c
>> @@ -256,12 +256,6 @@ static int parse_argv(int argc, char *argv[]) {
>>                          return 0;
>>
>>                  case ARG_PREFIX: {
>> -                        char *p;
>> -
>> -                        for (p = optarg; *p; p++)
>> -                                if (*p == '.')
>> -                                        *p = '/';
>> -
>
> Hm, are you sure this is right?

No, I was hoping that good people here would point me to the right
direction if I broke something and didn't know about it. Which you did,
thank you:)

> I mean, so far people could have just used the same format as they use
> in sysctl files (dots as separators). If we apply this, we break
> compatibility as we now expect filesystem paths. I think the better
> fix is to use normalize_sysctl() on the input. This allows both
> formats to be specified (relies on the fact that the root node does
> not contain dots or slashes in it's name).

Makes sense, I'll update the patch.

> Nevertheless, the documentation should clearly state which input is
> expected and the current code is definitely wrong as it only performs
> one way conversions.

Could you please point me to the right documentation that I can update?
My guess would be sysctl.d(5), since systemd-sysctl(8) refers to it.
Also, should I make it a separate patch?

> Thanks
> David

Thank you,
-- 
Jan Synacek
Software Engineer, Red Hat