[Mesa-dev] [PATCH] swr/rast: do not crash on NULL strings returned by getenv

[Emil Velikov]

On 18 September 2017 at 11:48, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> On Monday, 2017-09-18 11:29:21 +0100, Emil Velikov wrote:
>> From: Bernhard Rosenkraenzer <bero at lindev.ch>
>>
>> The current convinince function GetEnv feeds the results of getenv
>> directly into std::string(). That is a bad idea, since the variable
>> may be unset, thus we feed NULL into the C++ construct.
>>
>> The latter of which is not allowed and leads to a crash.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101832
>> Fixes: a25093de718 ("swr/rast: Implement JIT shader caching to disk")
>> Cc: Tim Rowley <timothy.o.rowley at intel.com>
>> Cc: Laurent Carlier <lordheavym at gmail.com>
>> Cc: Bernhard Rosenkraenzer <bero at lindev.ch>
>> [Emil Velikov: make an actual commit from the misc diff]
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>
> Laurent just sent this to the ML an hour before you, but this commit
> message is much better.
>
Guilty, sorry about that gents.

> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>
>> ---
>>  src/gallium/drivers/swr/rasterizer/core/utils.h | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/swr/rasterizer/core/utils.h b/src/gallium/drivers/swr/rasterizer/core/utils.h
>> index b096d2120cb..3c849e82d3b 100644
>> --- a/src/gallium/drivers/swr/rasterizer/core/utils.h
>> +++ b/src/gallium/drivers/swr/rasterizer/core/utils.h
>> @@ -365,7 +365,8 @@ static INLINE std::string GetEnv(const std::string& variableName)
>>      output.resize(valueSize - 1); // valueSize includes null, output.resize() does not
>>      GetEnvironmentVariableA(variableName.c_str(), &output[0], valueSize);
>>  #else
>> -    output = getenv(variableName.c_str());
>> +    char *o = getenv(variableName.c_str());
>> +    output = o ? std::string(o) : std::string();
>
> Like I mentioned [2] on his patch, I think this could be written
> better, eg:
>
>         char *env = getenv(variableName.c_str());
>         output = env ? env : "";
>
I'll let Tim and other SWR devs weight-in - I'm fine either way.

Thanks
Emil