This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2015年03月02日 16:50 by Hisham Muhammad, last changed 2022年04月11日 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| python-3.4.3-_posixsubprocess.c.fix.patch | Hisham Muhammad, 2015年03月02日 16:50 | Patch for Modules/_posixsubprocess.c fixing _sanity_check_python_fd_sequence | ||
| Messages (7) | |||
|---|---|---|---|
| msg237061 - (view) | Author: Hisham Muhammad (Hisham Muhammad) | Date: 2015年03月02日 16:50 | |
In Modules/_posixsubprocess.c, (the helper module for Lib/subprocess.py) there's a function called _sanity_check_python_fd_sequence which checks, as its comment says, if the fds in the sequence are all positive and sorted. The check to verify if it is sorted is incorrect (missing the update to the prev_fd variable), and also it is missing a test to see if fd's are repeated (which they shouldn't be, so the test should use <= rather than <). The attached patch, written against Python 3.4.3 source code, fixes it. |
|||
| msg237110 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2015年03月03日 07:34 | |
Haha, yes, that description and patch look correct. Thanks!
Fortunately this bug is low impact as this was just a sanity check and the calling code from subprocess.py was already passing the correct data in.
An ideal regression test: An explicit unittest that calls the _posixsubprocess API with some bad sequences of values in fds_to_keep and uses assertRaises to check for the appropriate ValueError("bad value(s) in fds_to_keep") coming out of it...
|
|||
| msg254699 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2015年11月15日 22:36 | |
Hisham, could you sign the Python contributor agreement? https://www.python.org/psf/contrib/contrib-form/ thanks! |
|||
| msg254701 - (view) | Author: Martin Panter (martin.panter) * (Python committer) | Date: 2015年11月16日 00:31 | |
FWIW I think this patch is trivial enough not to need an agreement. |
|||
| msg254709 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2015年11月16日 05:11 | |
heh, you're right. it's a trivial obvious fix for the mistake in the existing implementation. writing a test and committing. the other option would be to get rid of the sanity check entirely or change it not to use the odd "require a sorted list" code. but i like the sanity check and refactoring it to not require a sorted list within this code path would be complicated. |
|||
| msg254710 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2015年11月16日 05:29 | |
New changeset 55f7a99a5433 by Gregory P. Smith in branch '3.5': Fixes #23564: Fix a partially broken sanity check in the _posixsubprocess https://hg.python.org/cpython/rev/55f7a99a5433 New changeset 97e2a6810f7f by Gregory P. Smith in branch 'default': Fixes #23564: Fix a partially broken sanity check in the _posixsubprocess https://hg.python.org/cpython/rev/97e2a6810f7f |
|||
| msg254711 - (view) | Author: Gregory P. Smith (gregory.p.smith) * (Python committer) | Date: 2015年11月16日 05:30 | |
I didn't bother adding the fix to 3.4, it isn't high value. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:58:13 | admin | set | github: 67752 |
| 2015年11月16日 05:30:44 | gregory.p.smith | set | status: open -> closed resolution: fixed messages: + msg254711 versions: + Python 3.5, Python 3.6 |
| 2015年11月16日 05:29:54 | python-dev | set | nosy:
+ python-dev messages: + msg254710 |
| 2015年11月16日 05:11:16 | gregory.p.smith | set | messages: + msg254709 |
| 2015年11月16日 00:31:08 | martin.panter | set | nosy:
+ martin.panter messages: + msg254701 |
| 2015年11月15日 22:36:35 | gregory.p.smith | set | messages: + msg254699 |
| 2015年03月03日 07:34:45 | gregory.p.smith | set | priority: normal -> low messages: + msg237110 assignee: gregory.p.smith type: behavior stage: patch review |
| 2015年03月03日 02:40:29 | ned.deily | set | nosy:
+ gregory.p.smith |
| 2015年03月02日 16:50:12 | Hisham Muhammad | create | |