[Top][All Lists]

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

Re: ltdl bugs

From: Jeff Squyres
Subject: Re: ltdl bugs
Date: Sun, 3 Apr 2005 10:54:32 -0400

On Apr 3, 2005, at 10:17 AM, Ralf Wildenhues wrote:

I tried moving the three LT_DLFREE after the brace, and it /seems/ ok
then.  Am quite reluctant to put it in yet, because I fear to have
overlooked something.

So actually, I didn't do that for a specific reason: they're NULL in the self case.

Granted -- I only tested this on Linux; it's quite possible that they're not NULL in other cases. Easy to move these FREE's beyond the brace and protect them in "if" statements.

Ah, yes, here's the argument:

When we accept your patch, Jeff, we allow to let ltdl's view of loaded
modules get out of sync with what is really loaded. Now promise me that
we don't currently use any system API which disallows opening of an
already opened module. Nor that we will ever implement support for such
an API.

The general idea:
  /* later.. */

I don't feel competent to judge on this assumption.

Note it might be possible to do what you want inside lt_dlexit(), but
the amount of hair would grow.

Hmm. I'm not sure I understand this. Doesn't "resident" mean that the module is statically compiled in the application (i.e., no underlying dlopen() [or whatever] was required to load it)? If so:

- we're talking about the pre-sym case: presym_close() is a no-op, so it is safe to call this function, even in the self or pre-loaded case.

- we're also talking about the self case: on Linux, that's sys_dl_open(), which calls dlopen() with a filename==NULL (and gets a handle back). Technically, we should call dlclose() with that handle, too -- but the man page (on Linux) doesn't specify what happens in that scenario. Some tests on Linux and Solaris seem to show that this is ok (which makes sense -- if dlopen() allows you to open NULL, then it must allow you to close it, too). So it is safe to call this function for the self case.

- also, ltdl's view of loaded modules is out of sync with what is really loaded until you call lt_dlopen() the first time.

So my [ammended, per above] patch simply resets ltdl's view of what is loaded to what it was before you called lt_dlopen().

Is that correct? If so, my patch actually becomes much simpler -- there's no need to cordon off the unload module stuff -- just protect the DLFREE's with if statements:

--- ltdl.c.orig 2005-04-03 08:49:11.000000000 -0400
+++ ltdl.c      2005-04-03 10:53:16.000000000 -0400
@@ -3815,10 +3815,19 @@
       errors += unload_deplibs(handle);

       /* It is up to the callers to free the data itself.  */
-      LT_DLFREE (handle->caller_data);
-      LT_DLFREE (handle->info.filename);
-      LT_DLFREE (handle->info.name);
+      if (NULL != handle->caller_data)
+        {
+          LT_DLFREE (handle->caller_data);
+        }
+      if (NULL != handle->info.filename)
+        {
+          LT_DLFREE (handle->info.filename);
+        }
+      if (NULL != handle->info.name)
+        {
+          LT_DLFREE (handle->info.name);
+        }
       LT_DLFREE (handle);

       goto done;

Or I'm missing something...?

{+} Jeff Squyres
{+} address@hidden
{+} http://www.lam-mpi.org/

reply via email to

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