[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: dlloader documentation update
Re: dlloader documentation update
Mon, 25 Jan 2010 10:34:05 +0100
Thunderbird 126.96.36.199 (Windows/20090812)
Den 2010-01-24 11:59 skrev Ralf Wildenhues:
* Peter Rosin wrote on Fri, Jan 22, 2010 at 09:53:40AM CET:
So, I set out to update it, see attached patch.
When going through this, I found it strange that
lt_dlloader_add didn't call vtable->dlloader_init. Isn't
that desirable? I'll send an additional patch to show what
That one's already been shot down. ;->
Yes, I thought of that myself after sending the mail, but figured
I'd see if anyone else would catch it... ;-]
The comment about us needing testsuite exposure for this kind of code is
still very much valid though. We should try and see how far we can get
with the current documented sample code.
Not very far at all unless you propose to add some kind of compatibility
You'd have to make aliases so that "dlopen" == "lt_dlopen" *and*
"dlopen" == "lt_loadlibrary" (since "dlopen" is used for "host dependent
module loading"). How's that going to work on Cygwin that has both?
But ok, that's fairly trivial, and I suppose "dlopen" should get you
the lt_dlopen loader on Cygwin, but that overload of "dlopen" is
not supported by current ltdl.
Also, lt_dlloader_add (and others) have completely incompatible
signatures. There's no way around that. The documentation is just
too damn broken to have any relevance at all when there is
"competition" for some aspect of the api, if you ask me.
- the current code requires to pass in a structure allocated on the heap
(i.e., we keep a pointer to it); I'm inclined to want to fix ltdl by
copying the (documented part of the) input rather than change the
documented API though. Never mind the small memleak that ensues
(this should still have a NEWS entry however).
Making provisions for the neglected api documentation that introduce
an unaviodaable(?) memory leak is not the way to go, if you ask me.
- documenting the contents of a structure is bad bad bad. Since we
already do it, it is cast in stone and cannot be changed (neither
amended, because even that will cause old compiled code to break),
In this case I think it can because anyone trying to read the docs
to get something done will quickly see that the docs is just plain
broken. They would have to consult the code to get anything done.
Any user relying on docs (or anything really) that is so clearly
broken is just stupid.
Anyway, I did some data mining...
Commit 5451db06bd5391fa674846acba23b1a1ab778bc9 was "Okay to commit?"-ed
and it was apparently commited by the 72 hour rule, since I find
Anyway, that commit has no matching change in docs which is the origin
of the problem. So, the docs matches what the 1.5 series is doing, but
not the reality since 1.9b.
Here's a gentoo bug report for pulseaudio that seems releated:
Here's what they had to do in pulseaudio when they moved to libtool 2.2:
But they didn't get it right from the beginning so this is a fixup:
They had a few other changes in between as well...
See, people notice that ltdl has broken their code, and they somehow
pick up how to do it the new way w/o matching docs. They struggle a
bit though, and are not getting it right from the start, which is why
I think it would be better to just document the current api and be
done with it...
Also notice how they did use "dlopen" and how that was portable at
one point, but how they are now non-portable when they simply
converted that to "lt_dlopen".
But I don't get how to change the docs so that they suite you,
perhaps it's easiest if you did this part yourself?
- long deftype lines should be wrapped with @<newline>
I don't understand how to wrap the lines from that, please give
an example (or do it yourself, since I'm passing the buck anyway).
- as argued in the other thread, lt_dlloader_init should not be
documented nor used.
I have zapped that paragraph.
- typo: responible
That part and the perror removals look fairly ok to me, with
That's very little to fork out and commit for fairly limited value.
I changed "system dynamic loader" to "POSIX dynamic loader" for
I'll just attach my current patch and you can take it from there. I'll
volonteer for the testcase though, coming soonish...
They are in the crowd with the answer before the question.
> Why do you dislike Jeopardy?
Description: Text document