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

[Rowley, Timothy O]

I have a bit of a preference for Eric’s version.

-Tim

On Sep 18, 2017, at 7:10 AM, Emil Velikov <emil.l.velikov at gmail.com<mailto:emil.l.velikov at gmail.com>> wrote:

On 18 September 2017 at 11:48, Eric Engestrom <eric.engestrom at imgtec.com<mailto: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<mailto: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<mailto:timothy.o.rowley at intel.com>>
Cc: Laurent Carlier <lordheavym at gmail.com<mailto:lordheavym at gmail.com>>
Cc: Bernhard Rosenkraenzer <bero at lindev.ch<mailto:bero at lindev.ch>>
[Emil Velikov: make an actual commit from the misc diff]
Signed-off-by: Emil Velikov <emil.velikov at collabora.com<mailto: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<mailto: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
_______________________________________________
mesa-dev mailing list
mesa-dev at lists.freedesktop.org<mailto:mesa-dev at lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170919/c4dfad04/attachment-0001.html>