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

[David Herrmann]

Hi

On Mon, Sep 15, 2014 at 11:26 AM, Jan Synacek <jsynacek at redhat.com> wrote:
> David Herrmann <dh.herrmann at gmail.com> writes:
>> Hi
>>
>> On Mon, Sep 15, 2014 at 10:00 AM, Jan Synacek <jsynacek at redhat.com> wrote:
>>> David Herrmann <dh.herrmann at gmail.com> writes:
>>>> 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?
>>
>> I was referring to the --help text of the binary. There is no man-page
>> for it, so no need to update any of them.
>>
>> Feel free to put me on CC on v2 and I'll apply the patch.
>>
>> Thanks a lot!
>> David
>
> After tinkering with it for a while, I think that my original patch is
> correct. The prefix is a path prefix that is matched against actual
> paths, not variable names. That means that the prefix always has to
> start with a slash, otherwise nothing matches, which also means that
> normalizing would have no effect. And I think that the docstring doesn't
> need an update either. It says "Only apply rules that apply to *paths
> with the specified prefix*" (my emphasis added).

A path isn't necessarily a file-system path. With sysctl, we have to
ways to specify entries:

  1) You can specify them via legacy sysctl(2) names. These names use
dots as separators

  2) You can specify them via /proc paths. These use slashes as separators.

The conversion between 1) and 2) (both ways) replaces dots by slashes
and slashes by dots. However, you *always* have to convert both! That
is, if a sysctl name is "root.some.weird/entry", then this becomes
"root/some/weird.entry".

Now, the systemd-sysctl --prefix argument says it takes "paths",
however, it replaces dots with slashes, which made me assume it takes
legacy names instead. Nevertheless, this conversion is definitely
inconsistent, as it only replaces one part (dots->slashes), not both
(it will convert the example above to "root/some/weird/entry", which
is definitely wrong!). Hence, I suggested using normalize_sysctl(),
which does this properly.

Obviously, the result is compared against file-system paths. However,
that does not mean the input of --prefix is a file-system path. In
fact, given the current code it is very likely *NOT* a file-system
path. Otherwise, we wouldn't do the conversion.

The normalize_sysctl() helper uses some hack to allow both input
types: It relies on the first part of the path/name to be
alphanumeric. This is true for all sysctl nodes, therefore, we can
simply look for the first dot or slash and deduce the format. If it's
a slash, it's already a filesystem path, if it's a dot, it needs to be
converted. Thus, using normalize_sysctl() will allow using either
format with --prefix.

Does that make sense? If yes, I can write a patch and apply it.

Thanks
David