On 04-Jun-16 16:24, Martin Panter wrote:
> Martin Panter added the comment:
>
> Okay here are some more thoughts about your latest patch:
>
> ## Automatic RTLD_MEMBER ##
>
> I was still uneasy about the automatic setting of RTLD_MEMBER. But I looked for how others handle this, and I found Libtool’s LTDL library, and Apache Portable Runtime (APR). Both have a similar (but stricter) automatic addition based on detecting the archive(member) notation:
>
> http://git.savannah.gnu.org/cgit/libtool.git/commit/libltdl/loaders/dlopen.c?id=8fa719e
> https://github.com/apache/apr/commit/c27f8d2
The "stricter" from libtool is an excellent idea - I should done that as 
well, but I really hate string manipulation :)

I hope that the intent at least more clear, and should anyone ever 
complain about it not opening a file named: libFoo.a(libFOO.so) as a 
filename - by design, not accepted as a filename.
> So I guess I can accept this way, if you think it is better than my other ideas:
>
> # Always uses RTLD_MEMBER, never loads a plain file outside an archive
> name = "libcrypto.a(libcrypto.so.1.0.0)"
>
> # Other options, that could be returned by find_library() and would not conflict with a plain file name
> name = ("libcrypto.a", "libcrypto.so.1.0.0")  # (archive, member)
> name = ("libcrypto.a(libcrypto.so.1.0.0)", RTLD_MEMBER)  # (name, extra-flags)
>
> libcrypto = CDLL(name)
Isn't this more of an API change - name is no longer just a string?
>
> ## find_library() modes ##
>
> In your find_library() function, you still have three parts. Can you confirm that each behaviour is intended:
I have to catch a plane - will get back on these. Short - if I have a 
potential bug, then needs to be improved. More later.
>
> A) If I have a file called "crypto" in the current directory, find_library("crypto") returns "crypto". This does not seem right. On Linux, “gcc [. . .] -lcrypto” does not look for a file exactly called “crypto”.
>
> B) You are still stripping bits off the library name if it contains “lib” or a dot (.), so find_library("glib-2.0") is almost equivalent to find_library("b-2"). Isn’t this a bug?
>
> C) find_library("crypto") will return "/usr/lib/crypto" if such a file exists. Just like in A), this does not seem right to me.
>
> ## Other things ##
>
> * You don’t need to prefix most names with underscores, unless they could be confused with a public API. If you follow my earlier suggestion of renaming the new file to _aixutil.py (so it is obvious it is not a public module), then you can freely write “import re, os, sys”, etc.
>
> * No need to add the internal variable names to the function signatures. Just write find_library(name), and if you need to initialize a variable, do that in the body.
>
> * I suggest to go over all the regular expressions, and either change them to plain string searching, or make sure special characters and external variables are escaped as necessary. A comment explaining what the RE is trying to do might help too.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue26439>
> _______________________________________