[Python-Dev] [Python-checkins] cpython (3.2): Fixes issue #8052: The posix subprocess module's close_fds behavior was (original) (raw)

Gregory P. Smith greg at krypto.org
Sun Jan 22 10:08:13 CET 2012

On Sat, Jan 21, 2012 at 4:21 PM, Benjamin Peterson <benjamin at python.org> wrote:

2012/1/21 gregory.p.smith <python-checkins at python.org>: ...

+/* Convert ASCII to a positive int, no libc call. no overflow. -1 on error. */ Is no libc call important?

Yes. strtol() is not on the async signal safe C library function list.

+static int posintfromascii(char *name) To be consistent with the rest of posixmodule.c, "static int" should be on a different line from the signature. This also applies to all other function declarations added by this.

Python C style as a whole, yes. This file already has a mix of same line vs two line declarations, I added these following the style of the functions immediately surrounding them. Want a style fixup on the whole file?

+{ +    int num = 0; +    while (*name >= '0' && *name <= '9') { +        num = num * 10 + (*name - '0'); +        ++name; +    } +    if (*name) +        return -1;  /* Non digit found, not a number. */ +    return num; +} + + +/* Returns 1 if there is a problem with fdsequence, 0 otherwise. */ +static int sanitycheckpythonfdsequence(PyObject *fdsequence) +{ +    Pyssizet seqidx, seqlen = PySequenceLength(fdsequence); PySequenceLength can fail.

It has already been checked not to by the only entry point into the code in this file.

+    long prevfd = -1; +    for (seqidx = 0; seqidx < seqlen; ++seqidx) {_ _+        PyObject* pyfd = PySequenceFastGETITEM(fdsequence, seqidx);_ _+        long iterfd = PyLongAsLong(pyfd);_ _+        if (iterfd < 0 || iterfd < prevfd || iterfd > INTMAX) { +            /* Negative, overflow, not a Long, unsorted, too big for a fd. */ +            return 1; +        } +    } +    return 0; +} + + +/* Is fd found in the sorted Python Sequence? */ +static int isfdinsortedfdsequence(int fd, PyObject *fdsequence) +{ +    /* Binary search. */ +    Pyssizet searchmin = 0; +    Pyssizet searchmax = PySequenceLength(fdsequence) - 1; +    if (searchmax < 0) +        return 0; +    do { +        long middle = (searchmin + searchmax) / 2; +        long middlefd = PyLongAsLong( +                PySequenceFastGETITEM(fdsequence, middle)); No check for error?

_sanity_check_python_fd_sequence() already checked the entire list to guarantee that there would not be any such error.

+        if (fd == middlefd) +            return 1; +        if (fd > middlefd) +            searchmin = middle + 1; +        else +            searchmax = middle - 1; +    } while (searchmin <= searchmax); +    return 0; +}

In general this is an extension module that is best viewed as a whole including its existing comments rather than as a diff.

It contains code that will look "odd" in a diff because much of it executes in a path where not much is allowed (post fork, pre exec) and no useful way of responding to an error is possible so it attempts to pre-check for any possible errors up front so that later code that is unable to handle errors cannot possibly fail.

-gps

More information about the Python-Dev mailing list