aixtools added the comment:
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 <rep...@bugs.python.org>
> <http://bugs.python.org/issue26439>
> _______________________________________
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue26439>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
