Author martin.panter
Recipients David.Edelsohn, Michael.Felt, aixtools@gmail.com, martin.panter
Date 2016-06-04.14:24:00
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1465050241.59.0.841731746859.issue26439@psf.upfronthosting.co.za>
In-reply-to
Content
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

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)

## find_library() modes ##

In your find_library() function, you still have three parts. Can you confirm that each behaviour is intended:

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.
History
Date User Action Args
2016-06-04 14:24:01martin.pantersetrecipients: + martin.panter, David.Edelsohn, Michael.Felt, aixtools@gmail.com
2016-06-04 14:24:01martin.pantersetmessageid: <1465050241.59.0.841731746859.issue26439@psf.upfronthosting.co.za>
2016-06-04 14:24:01martin.panterlinkissue26439 messages
2016-06-04 14:24:00martin.pantercreate