Re: [PATCH 2/4] Initialize tunable list with the GLIBC_TUNABLES environment variable

[Florian Weimer <fweimer at redhat dot com>]

On 11/16/2016 09:35 AM, Siddhesh Poyarekar wrote:

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c

> +static char *
> +tunables_strdup (const char *in)
> +{
> +  size_t i = 0;
> +
> +  while (in[i++]);

Please use an explicit comparison against '\0'.

> +  char *out = __mmap (NULL, ALIGN_UP (i, __getpagesize ()),
> +		      PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
> +		      0);
> +
> +  if (out == MAP_FAILED)
> +    return NULL;

See my initial mail.  errno and __getpagesize are not available at this 
point.

> +static size_t
> +min_strlen (const char *s)
> +{
> +  size_t i = 0;
> +  while (*s++ != '\0')
> +    i++;
> +
> +  return i;
> +}

Is there anything which ensures that GCC does not replace this 
implementation with strlen?

>  /* Disable a tunable if it is set.  */
>  static void
>  disable_tunable (tunable_id_t id, char **envp)
> @@ -216,6 +318,23 @@ disable_tunable (tunable_id_t id, char **envp)
>
>    if (env_alias)
>      tunables_unsetenv (envp, tunable_list[id].env_alias);
> +
> +  char *tunable = getenv (GLIBC_TUNABLES);
> +  const char *cmp = tunable_list[id].name;
> +  const size_t len = min_strlen (cmp);
> +
> +  while (tunable && *tunable != '\0' && *tunable != ':')
> +    {
> +      if (is_name (tunable, cmp))
> +	{
> +	  tunable += len;
> +	  /* Overwrite the = and the value with colons.  */
> +	  while (*tunable != '\0' && *tunable != ':')
> +	    *tunable++ = ':';
> +	  break;
> +	}
> +      tunable++;
> +    }
>  }

This assumes that the process environment is not mapped read-only.  I'm 
not sure if this is guaranteed by the ABI.

Thanks,
Florian