The check for whether to request more data was all messed up.
On the client this needs to be after initial metadata is sent to guarantee that we have a live stream, but there's no such requirement at the server. Fix this.
I also ran clang-format across the code so there are some formatting changes.
- Use METH_O and METH_NOARGS where appropriate, modifying the C functions
appropriately. METH_O is for functions that take a single PyObject, and
it's passed directly instead of 'args'. METH_NOARGS is for functions
that take no arguments, and they get called with just one argument
('self'.)
- In PyArg_ParseTuple*() calls, specify the callable's name for more
descriptive exception messages.
- For tp_init functions (which always take keyword arguments) introduce
keyword argument parsing (using the C local variables as keywords,
although I don't know if they're the best names to use.) This is mostly
as a way to show how keyword arguments are done in C. An alternative
method is to use _PyArg_NoKeywords(kwds) (see
https://hg.python.org/cpython/file/70a55b2dee71/Python/getargs.c#l1820,
but unfortunately it's not part of the official API,) or check manually
that the dict is empty.
- Check the return value of Python API functions that can return an error
indicator (NULL or -1.) PyFloat_AsDouble is also one of these, but we
don't check the return type (we would have to compare the result to 1.0
with ==, which is not a thing you should do) so just call PyErr_Occurred
unconditionally.
- Change Py_BuildValue() calls with just "O" formats into PyTuple_Pack
calls. It requires less runtime checking.
- Replace Py_BuildValue()/PyObject_CallObject pairs with
PyObject_CallFunctionObjArgs (since all of them have just PyObject*
arguments.) If the Py_BuildValue formats had included other types,
PyObject_CallFunction() would have been easier, but no need in these
cases.
- Replace Py_BuildValue("f", ...) with PyFloat_FromDouble(...). Less
runtime checking and parsing necessary, and more obvious in what it does.
- In the PyType structs, replace "PyObject_HEAD_INIT(NULL) 0" with
"PyVarObject_HEAD_INIT(NULL, 0)". Anything with an ob_size struct member
is a PyVarObject, although the distinction isn't all that import; it's
just a more convenient macro.
- Assign tp_new in the PyType structs directly, like all other struct
members, rather than right before the PyType_Ready() call.
- Remove PyErr_SetString() calls in places that already have a (meaningful)
exception set.
- Add a PyErr_Format() for an error return that wasn't setting an exception
(PyObject_TypeCheck() doesn't set an exception.)
- Remove NULL assignments to struct members in the error paths of the
tp_init functions. PyObject structs are always zeroed after allocation,
guaranteed. (If there's a way for them to already contain an object
you'd use Py_CLEAR() to clear them, but that can't happen in these
cases.)
- Remove a few unnecessary parentheses.