[PATCH 6/8] util: Check for errors in virLogSetFromEnv

[Erik Skultety]

On Tue, Jan 04, 2022 at 02:47:10PM +0100, Martin Kletzander wrote:
> And make callers check the return value as well.  This helps error out early for
> invalid environment variables.
> 
> That is desirable because it could lead to deadlocks.  This can happen when
> resetting logging after fork() reports translated errors because gettext
> functions are not reentrant.  Well, it is not limited to resetting logging after
> fork(), it can be any translation at that phase, but parsing environment
> variables is easy to make fail on purpose to show the result, it can also happen
> just due to a typo.


> Logging settings are also something that we want to report
> errors on for example when it is being done over admin API.

True in general, but slightly off-topic wrt to the patch itself as
virLogSetFromEnv is irrelevant to admin API usage.

...

> -void
> +int
>  virLogSetFromEnv(void)
>  {
>      const char *debugEnv;
>  
>      if (virLogInitialize() < 0)
> -        return;
> +        return -1;
>  
>      debugEnv = getenv("LIBVIRT_DEBUG");
> -    if (debugEnv && *debugEnv)
> -        virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv));
> +    if (debugEnv && *debugEnv) {
> +        int priority = virLogParseDefaultPriority(debugEnv);
> +        if (priority < 0 ||
> +            virLogSetDefaultPriority(priority) < 0)
> +        return -1;

           ^^^ indentation

> +    }
>      debugEnv = getenv("LIBVIRT_LOG_FILTERS");
> -    if (debugEnv && *debugEnv)
> -        virLogSetFilters(debugEnv);
> +    if (debugEnv && *debugEnv &&
> +        virLogSetFilters(debugEnv))
> +        return -1;
>      debugEnv = getenv("LIBVIRT_LOG_OUTPUTS");
> -    if (debugEnv && *debugEnv)
> -        virLogSetOutputs(debugEnv);
> +    if (debugEnv && *debugEnv &&
> +        virLogSetOutputs(debugEnv))
> +        return -1;
> +
> +    return 0;
>  }

Reviewed-by: Erik Skultety <eskultet at redhat.com>