Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(6)

#22636: avoid using a shell in ctypes.util: replace os.popen with subprocess

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 7 months ago by vstinner
Modified:
3 years, 11 months ago
Reviewers:
pitrou, storchaka, vadmium+py, storchaka+cpython
CC:
barry, haypo, Arfrever, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka, koobs
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 23

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Patch Set 8 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/ctypes/test/test_find.py View 1 2 3 4 5 6 7 2 chunks +7 lines, -1 line 0 comments Download
Lib/ctypes/util.py View 1 2 3 4 5 6 7 7 chunks +82 lines, -45 lines 4 comments Download
Misc/NEWS View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 10
pitrou_free.fr
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode116 Lib/ctypes/util.py:116: except FileNotFoundError: There shouldn't be a FileNotFoundError. Besides, you ...
5 years, 7 months ago #1
victor.stinner_gmail.com
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode116 Lib/ctypes/util.py:116: except FileNotFoundError: On 2014/10/16 13:46:57, AntoinePitrou wrote: > There ...
5 years, 7 months ago #2
storchaka_gmail.com
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode92 Lib/ctypes/util.py:92: expr = os.fsencode(r'[^\(\)\s]*lib%s\.[^\(\)\s]*' % name) Why removed re.escape()? The ...
5 years, 7 months ago #3
Martin Panter
https://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: return None On 2014/10/22 13:52:15, storchaka wrote: > I ...
4 years ago #4
storchaka_gmail.com
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: return None On 2016/05/08 06:42:34, vadmium wrote: > On ...
4 years ago #5
Martin Panter
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: return None On 2016/05/08 18:10:39, storchaka wrote: > On ...
4 years ago #6
storchaka_gmail.com
http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: return None On 2016/05/09 02:51:41, vadmium wrote: > On ...
4 years ago #7
Martin Panter
https://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/22636/diff/13091/Lib/ctypes/util.py#newcode92 Lib/ctypes/util.py:92: expr = os.fsencode(r'[^\(\)\s]*lib%s\.[^\(\)\s]*' % name) On 2014/10/22 13:52:15, storchaka ...
4 years ago #8
storchaka
http://bugs.python.org/review/22636/diff/17636/Lib/ctypes/util.py File Lib/ctypes/util.py (right): http://bugs.python.org/review/22636/diff/17636/Lib/ctypes/util.py#newcode96 Lib/ctypes/util.py:96: cmd = 'if type gcc >/dev/null 2>&1; then CC=gcc; ...
3 years, 11 months ago #9
Martin Panter
3 years, 11 months ago #10
http://bugs.python.org/review/22636/diff/17636/Lib/ctypes/util.py
File Lib/ctypes/util.py (right):

http://bugs.python.org/review/22636/diff/17636/Lib/ctypes/util.py#newcode96
Lib/ctypes/util.py:96: cmd = 'if type gcc >/dev/null 2>&1; then CC=gcc; elif
type cc >/dev/null 2>&1; then CC=cc;else exit; fi;' \
On 2016/06/15 19:32:11, storchaka wrote:
> What if neither gcc nor cc is found?

In this case, the shell script should exit without any stdout, nothing useful is
found in trace, and the function returns None. Same behaviour as before.
Remember the exit 10 never triggered the rv == 10 check, so there is no
behaviour change.

http://bugs.python.org/review/22636/diff/17636/Lib/ctypes/util.py#newcode127
Lib/ctypes/util.py:127: with null:
On 2016/06/15 19:32:11, storchaka wrote:
> null is closed before proc.communicate() is executed.

That is okay: we only need null open to pass to the child’s stderr. Once the
child is spawned, the parent can close its copy.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+