[Top][All Lists]

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

Re: lt_dlopenadvise() inconsistencies in libtool-2.2.2

From: Gary V . Vaughan
Subject: Re: lt_dlopenadvise() inconsistencies in libtool-2.2.2
Date: Sun, 20 Apr 2008 16:21:44 -0400

Hi Gary,

On 10 Apr 2008, at 18:00, Gary Kumfert wrote:
I am reporting either a documentation bug, an implementation bug,
or both in 2.2.2.  I'm not sure what the intent was, but it took
me (and my good friend, Tom) a bit of time to hunt
down why my libraries weren't being loaded globally like they
used to when I upgraded libtool.

Thanks for the excellent in depth report.  Sorry it took me a while to
get to it while we migrated libtool from CVS into git.

The code says:
        lt_dlopenadvise() takes a lt_dladvise as its
        second argument (not a lt_dladvise* as with
        all the other lt_dladvise_* functions!)

And this is also what the testsuite checks is working in tests/ lt_dladvise.at.

The documentation says:
1.  Actually Section 11.1 doesn't have an entry for lt_dlopenadvise()
        which is a bug in itself.

Worse, I corrected this by extrapolating some documentation for lt_dlopenadvise from the bogus example in the documentation. In this case the documentation was wrong to start with, and is now even more wrong. I'm preparing a patch at the moment.

2. AND the code examples where lt_dlopenadvise() appears
        (e.g. lt_dladvise_ext() in Section 11.1) indicate a lt_dladvise*
        which doesn't gibe with the code.
         my_dlopenext (const char *filename)
           lt_dlhandle handle = 0;
           lt_dladvise advise;

if (!lt_dladvise_init (&advise) && !lt_dladvise_ext (&advise)) handle = lt_dlopenadvise (filename, &advise); // <<<<---- wrong!!!

           lt_dladvise_destroy (&advise);

           return handle;

Also wrong in the documentation, and now corrected in the forthcoming patch.

See the problem?

1. What was the intended API?  lt_dladvise or lt_dladvise* ?

lt_dlopenadvise does not change the contents of lt_dladvise, and thus accepts the type directly rather than a pointer to it. The other API calls potentially give back new memory (or even delete the memory at the address passed) and thus require
a pointer to an lt_dladvise.

2. Why bother with all the casting in the implementation?
  The type is known statically and the compiler *could*
  have helped in finding this problem if we let it.

Mostly for consistency with how lt_dlhandles are manipulated. The idea is that outside libltdl implementation code only a "void *" type is available, and callers cannot play with the internals of the struct. However, when the implementation wants to access the internals it must cast the void * address back to a struct

I agree that this was arguably not the best decision, and maybe once we've branched the tree for 2.2 maintenance we should consider putting the struct in a public header along with a warning that it should not be accessed to give
the compiler the best chance to catch these kinds of errors in future.

3. What is the resolution?  I'd like to know what to plan for.

Implementation beats documentation.  Patch forthcoming.

  ())_.              Email me: address@hidden
  ( '/           Read my blog: http://blog.azazil.net
  / )=         ...and my book: http://sources.redhat.com/autobook

Attachment: PGP.sig
Description: This is a digitally signed message part

reply via email to

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