[systemd-devel] [PATCHv2] sysctl: consider --prefix while parsing the files

[Zbigniew Jędrzejewski-Szmek]

On Thu, Feb 05, 2015 at 03:37:13PM +0100, Umut Tezduyar Lindskog wrote:
> On Wed, Feb 4, 2015 at 4:55 PM, Zbigniew Jędrzejewski-Szmek
> <zbyszek at in.waw.pl> wrote:
> > On Wed, Feb 04, 2015 at 03:50:01PM +0100, Umut Tezduyar Lindskog wrote:
> >> not while applying the parsed sysctl values. Otherwise
> >> info "Overwriting earlier assignment of %s in file %s" is
> >> visible many times even though the given --prefix doesn't
> >> try to set the overridden value.
> >> ---
> >>  src/sysctl/sysctl.c | 32 ++++++++++++++++----------------
> >>  1 file changed, 16 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c
> >> index 973e67e..b22aff5 100644
> >> --- a/src/sysctl/sysctl.c
> >> +++ b/src/sysctl/sysctl.c
> >> @@ -78,22 +78,6 @@ static int apply_sysctl(const char *property, const char *value) {
> >>          n = stpcpy(p, "/proc/sys/");
> >>          strcpy(n, property);
> >>
> >> -        if (!strv_isempty(arg_prefixes)) {
> >> -                char **i;
> >> -                bool good = false;
> >> -
> >> -                STRV_FOREACH(i, arg_prefixes)
> >> -                        if (path_startswith(p, *i)) {
> >> -                                good = true;
> >> -                                break;
> >> -                        }
> >> -
> >> -                if (!good) {
> >> -                        log_debug("Skipping %s", p);
> >> -                        return 0;
> >> -                }
> >> -        }
> >> -
> >>          k = write_string_file(p, value);
> >>          if (k < 0) {
> >>                  log_full(k == -ENOENT ? LOG_DEBUG : LOG_WARNING,
> >> @@ -173,6 +157,22 @@ static int parse_file(Hashmap *sysctl_options, const char *path, bool ignore_eno
> >>                  p = normalize_sysctl(strstrip(p));
> >>                  value = strstrip(value);
> >>
> >> +                if (!strv_isempty(arg_prefixes)) {
> >> +                        char **i, *t;
> >> +                        bool good = false;
> >> +                        STRV_FOREACH(i, arg_prefixes) {
> >> +                                t = path_startswith(*i, "/proc/sys/");
> >> +                                if (t == NULL)
> >> +                                        t = *i;
> >> +                                if (path_startswith(p, t)) {
> >> +                                        good = true;
> >> +                                        break;
> >> +                                }
> >> +                        }
> >> +                        if (!good)
> >> +                                continue;
> >> +                }
> > While at it, wouldn't it be better to use a goto and do away with the
> > good variable. This will give a diff of -7/+3, a win also for readability imho.
> 
> How Zbyszek. I am confused.


       if (!strv_isempty(arg_prefixes)) {
               char **i, *t;

               STRV_FOREACH(i, arg_prefixes) {
                       t = path_startswith(*i, "/proc/sys/");
                       if (t == NULL)
                               t = *i;
                       if (path_startswith(p, t))
       		           goto found;
               }
       	   /* not found */
       	   continue;
       }
found:
       ...

Zbyszek