[Python-Dev] Should we fix these errors?

[Chris Angelico]

On Sat, Jul 23, 2016 at 12:36 AM, Guido van Rossum <guido at python.org> wrote:
> Somebody did some research and found some bugs in CPython (IIUC). The
> published some questionable fragments. If there's a volunteer we could
> probably easily fix these. (I know we already have occasional Coverity
> scans and there are other tools too (anybody try lgtm yet?) But this
> seems honest research (also Python leaves Ruby in the dust :-):
>
> http://www.viva64.com/en/b/0414/

First and foremost: All of these purported bugs appear to have been
found by compiling on Windows. Does Coverity test a Windows build? If
not, can we get it to? These look like the exact types of errors that
Coverity *would* discover.

Fragment N1 is accurate in current Python. (Although the wording of
the report leaves something to be desired. "The SOCKET type in Windows
is unsigned, so comparing it against null is meaningless." - only "x <
0" (not null) is meaningless.) It's lines 1702 and 2026 in current
Python. What's the best solution? Create a macro VALID_SOCKET with two
different definitions, one using "x < 0" and the other using "x !=
INVALID_SOCKET"?

Fragment N2 doesn't appear to be in CPython 3.6 though. I can't find a
file called a_print.c, nor anything with ASN1_PRINTABLE_type in it.
Third party code? 2.7 only? I've no idea.

(It'd be so much more helpful if file paths had been given instead of
just fragment codes. The error messages include file names without
paths in them.)

Fragment N3: Looks like a legit issue.
http://bugs.python.org/issue27591 created with patch.

Fragment N4, N5, N6a: Can't find bn_lib.c, dh_ameth.c, or cms_env.c in
the cpython tree anywhere. Google suggests that they could be part of
OpenSSL (which could be true of a_print.c from N2). Does Python bundle
any OpenSSL source anywhere?

Fragment N6b (there's a completely unrelated issue paired up in N6): I
don't understand all of what's being said here. The error message
quoted refers to _elementtree.c:917, which is an understandable false
positive for the static checker; the problem can't happen, though,
because line 913 checks for NULL and will construct a new empty list,
and line 916 iterates up to the new list's length, so line 917 can
never be reached if self->extra is NULL. But their analyzer can't know
that. On the other hand, the paragraph and code snippet are referring
to _PyState_AddModule in Modules/pystate.c, which is never called with
def=NULL anywhere else in CPython; unless it's intended to be public,
the check on line 292 could simply be removed.

Conclusion: CPython may need some better static checking in Windows
mode, but probably not desperately enough to buy their product (which
is presumably the point of that blog).

ChrisA