[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Error reporting should be improved

From: Bob Friesenhahn
Subject: Re: Error reporting should be improved
Date: Wed, 30 Dec 2009 10:37:49 -0600 (CST)
User-agent: Alpine 2.01 (GSO 1266 2009-07-14)

On Wed, 30 Dec 2009, Peter Rosin wrote:

There used to be some useless effort to make it thread safe. Apparently that is gone now. Regardless, there is also a potential re-entrancy issue, which could be encountered if a legacy OS interface is supported (re-implemented) via dlopen() or if some other implementation logic (e.g. lazy symbol resolution) uses dlsym().

I can't say that I understand what you are describing, but dlerror need
not be reentrant[1], and we do various manipulations of internal state
during lt_dlopen etc, so I don't see how any argument about reentrancy
can hold. What am I missing this time?

What I am saying is that an OS may choose to re-implement its legacy loader interfaces via dlopen() and since we still provide a loader driver for the legacy interface, use of this other loader may scramble the error reports from the dlopen() loader.

This patch does of course not address the original issue, it only
addresses the issue with dlerror returning a string to the ltdl user
that might have a limited life span.

The patch does not seem to address that since it does not allocate a buffer to store the error string. What it does address is how to collect multiple error messages.

Unfortunately, lt_dladderror() is publically exposed so it is difficult to change its arguments even though it is not documented.

This patch does not change lt_dladderror in a way that I classify as
significant. Do you disagree?

I don't disagree, but I had suggested that it is quite useful to know the origin of the error report. The origin could have been handled via another parameter.

I have not found anything in your message that "kills" the patch, but
yet I get the feeling that you do not like it, and I would like to know
what you think is wrong with it...

It seems to me that these error strings need to be in allocated buffer storage so that they don't point to potentially volatile storage. Also, there needs to be a way to clear out the existing errors when a lt_dlopen*() or lt_dlsym() starts so that the errors only correspond to the last requested operation.

The current implementation appears to be a memory leak since nothing is freeing the error string pointer (user_error_strings).

Bob Friesenhahn
address@hidden, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,    http://www.GraphicsMagick.org/

reply via email to

[Prev in Thread] Current Thread [Next in Thread]