On Thu, Nov 13, 2014 at 6:22 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Thu, Nov 13, 2014 at 6:43 PM, Aaron Watry <awa...@gmail.com> wrote: >> dlopen allocates a string on dlopen failure which is retrieved via dlerror. >> In >> order to free that string, you need to retrieve and then free it. > > Are you basically saying that glibc leaks memory and you're trying to > make up for it? What if you use a non-buggy library? Or is dlopen() > specified in such a way that if it fails, you must free the result of > dlerror? I see nothing in the man pages to suggest that...
The closest that I can come to documentation at least implying this is [1]: " RETURN VALUE If file cannot be found, cannot be opened for reading, is not of an appropriate object format for processing by dlopen(), or if an error occurs during the process of loading file or relocating its symbolic references, dlopen() shall return NULL. More detailed diagnostic information shall be available through dlerror(). " Which implies that libdl needs to keep some sort of state regarding the last error encountered. I see no requirement that it keep a malloc'd string, just that it keep some state information around. [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/dlopen.html That does seem to lead one to read the dlerror() page that has this gem: " DESCRIPTION The dlerror() function shall return a null-terminated character string (with no trailing <newline>) that describes the last error that occurred during dynamic linking processing. If no dynamic linking errors have occurred since the last invocation of dlerror(), dlerror() shall return NULL. Thus, invoking dlerror() a second time, immediately following a prior invocation, shall result in NULL being returned. <snip> APPLICATION USAGE The messages returned by dlerror() may reside in a static buffer that is overwritten on each call to dlerror() " So, it may or may not return a malloc'd string, and all I've managed here is to fix a leak in glibc's specific implementation. The above docs seem to imply that dlopen() triggering an error needs to populate some state and dlerror() retrieves the last error that has occurred since the last dlerror() call. calling dlerror() again at that point will return null. That being said, I think a simpler fix and probably more correct fix would be to do: dlerror(); dlerror(); Thoughts? --Aaron > >> >> In order to keep things legit the windows/other util_dl_error paths allocate >> and then copy their error message into a buffer as well. >> >> Signed-off-by: Aaron Watry <awa...@gmail.com> >> CC: Ilia Mirkin <imir...@alum.mit.edu> >> >> v3: Switch comment to C-Style >> v2: Use strdup instead of calloc/strcpy >> --- >> src/gallium/auxiliary/pipe-loader/pipe_loader.c | 5 +++++ >> src/gallium/auxiliary/util/u_dl.c | 4 ++-- >> 2 files changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.c >> b/src/gallium/auxiliary/pipe-loader/pipe_loader.c >> index 8e79f85..7a4e0b1 100644 >> --- a/src/gallium/auxiliary/pipe-loader/pipe_loader.c >> +++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.c >> @@ -25,6 +25,8 @@ >> * >> **************************************************************************/ >> >> +#include <dlfcn.h> >> + >> #include "pipe_loader_priv.h" >> >> #include "util/u_inlines.h" >> @@ -101,6 +103,9 @@ pipe_loader_find_module(struct pipe_loader_device *dev, >> if (lib) { >> return lib; >> } >> + >> + /* Retrieve the dlerror() str so that it can be freed properly */ >> + FREE(util_dl_error()); >> } >> } >> >> diff --git a/src/gallium/auxiliary/util/u_dl.c >> b/src/gallium/auxiliary/util/u_dl.c >> index aca435d..00c4d7c 100644 >> --- a/src/gallium/auxiliary/util/u_dl.c >> +++ b/src/gallium/auxiliary/util/u_dl.c >> @@ -87,8 +87,8 @@ util_dl_error(void) >> #if defined(PIPE_OS_UNIX) >> return dlerror(); >> #elif defined(PIPE_OS_WINDOWS) >> - return "unknown error"; >> + return strdup("unknown error"); >> #else >> - return "unknown error"; >> + return strdup("unknown error"); >> #endif >> } >> -- >> 2.1.0 >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev