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

[Martin Kletzander]

On Wed, Jan 05, 2022 at 12:29:18PM +0100, Erik Skultety wrote:
>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.
>

Yeah, this was supposed to be a part of another commit message, I just
wanted to guard against someone suggesting we do not report errors at
all (which would be another solution, but a wrong one I think).

>...
>
>> -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
>

Thanks for catching that!

>> +    }
>>      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>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220105/e62e64bc/attachment-0001.sig>