From c04c573fb3ef294362d193b84249498eb4c8ef91 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Thu, 19 Feb 2015 13:37:08 -0800 Subject: Clean up Python C API use in src/python/src/grpc/_adapter/*.c. Specifically: - 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. --- src/python/src/grpc/_adapter/_c.c | 9 +- src/python/src/grpc/_adapter/_call.c | 59 ++++----- src/python/src/grpc/_adapter/_channel.c | 12 +- src/python/src/grpc/_adapter/_completion_queue.c | 132 ++++++++++++--------- src/python/src/grpc/_adapter/_server.c | 36 +++--- src/python/src/grpc/_adapter/_server_credentials.c | 21 ++-- 6 files changed, 145 insertions(+), 124 deletions(-) diff --git a/src/python/src/grpc/_adapter/_c.c b/src/python/src/grpc/_adapter/_c.c index 13eb93fe5a..55b9d0512c 100644 --- a/src/python/src/grpc/_adapter/_c.c +++ b/src/python/src/grpc/_adapter/_c.c @@ -40,19 +40,20 @@ #include "grpc/_adapter/_server.h" #include "grpc/_adapter/_server_credentials.h" -static PyObject *init(PyObject *self, PyObject *args) { +static PyObject *init(PyObject *self) { grpc_init(); Py_RETURN_NONE; } -static PyObject *shutdown(PyObject *self, PyObject *args) { +static PyObject *shutdown(PyObject *self) { grpc_shutdown(); Py_RETURN_NONE; } static PyMethodDef _c_methods[] = { - {"init", init, METH_VARARGS, "Initialize the module's static state."}, - {"shut_down", shutdown, METH_VARARGS, + {"init", (PyCFunction)init, METH_NOARGS, + "Initialize the module's static state."}, + {"shut_down", (PyCFunction)shutdown, METH_NOARGS, "Shut down the module's static state."}, {NULL}, }; diff --git a/src/python/src/grpc/_adapter/_call.c b/src/python/src/grpc/_adapter/_call.c index 7e62c1b7a3..325d3d5bbd 100644 --- a/src/python/src/grpc/_adapter/_call.c +++ b/src/python/src/grpc/_adapter/_call.c @@ -46,10 +46,11 @@ static int pygrpc_call_init(Call *self, PyObject *args, PyObject *kwds) { const char *method; const char *host; const double deadline; + static char *kwlist[] = {"channel", "method", "host", "deadline", NULL}; - if (!PyArg_ParseTuple(args, "O!ssd", &pygrpc_ChannelType, &channel, &method, - &host, &deadline)) { - self->c_call = NULL; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!ssd:Call", kwlist, + &pygrpc_ChannelType, &channel, &method, + &host, &deadline)) { return -1; } @@ -77,7 +78,7 @@ static const PyObject *pygrpc_call_invoke(Call *self, PyObject *args) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "O!OO", &pygrpc_CompletionQueueType, + if (!(PyArg_ParseTuple(args, "O!OO:invoke", &pygrpc_CompletionQueueType, &completion_queue, &metadata_tag, &finish_tag))) { return NULL; } @@ -103,7 +104,7 @@ static const PyObject *pygrpc_call_write(Call *self, PyObject *args) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "s#O", &bytes, &length, &tag))) { + if (!(PyArg_ParseTuple(args, "s#O:write", &bytes, &length, &tag))) { return NULL; } @@ -123,15 +124,10 @@ static const PyObject *pygrpc_call_write(Call *self, PyObject *args) { return result; } -static const PyObject *pygrpc_call_complete(Call *self, PyObject *args) { - const PyObject *tag; +static const PyObject *pygrpc_call_complete(Call *self, PyObject *tag) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "O", &tag))) { - return NULL; - } - call_error = grpc_call_writes_done_old(self->c_call, (void *)tag); result = pygrpc_translate_call_error(call_error); @@ -147,7 +143,7 @@ static const PyObject *pygrpc_call_accept(Call *self, PyObject *args) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "O!O", &pygrpc_CompletionQueueType, + if (!(PyArg_ParseTuple(args, "O!O:accept", &pygrpc_CompletionQueueType, &completion_queue, &tag))) { return NULL; } @@ -164,21 +160,16 @@ static const PyObject *pygrpc_call_accept(Call *self, PyObject *args) { return result; } -static const PyObject *pygrpc_call_premetadata(Call *self, PyObject *args) { +static const PyObject *pygrpc_call_premetadata(Call *self) { /* TODO(b/18702680): Actually support metadata. */ return pygrpc_translate_call_error( grpc_call_server_end_initial_metadata_old(self->c_call, 0)); } -static const PyObject *pygrpc_call_read(Call *self, PyObject *args) { - const PyObject *tag; +static const PyObject *pygrpc_call_read(Call *self, PyObject *tag) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "O", &tag))) { - return NULL; - } - call_error = grpc_call_start_read_old(self->c_call, (void *)tag); result = pygrpc_translate_call_error(call_error); @@ -198,16 +189,30 @@ static const PyObject *pygrpc_call_status(Call *self, PyObject *args) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "OO", &status, &tag))) { + if (!(PyArg_ParseTuple(args, "OO:status", &status, &tag))) { return NULL; } code = PyObject_GetAttrString(status, "code"); + if (code == NULL) { + return NULL; + } details = PyObject_GetAttrString(status, "details"); + if (details == NULL) { + Py_DECREF(code); + return NULL; + } c_code = PyInt_AsLong(code); - c_message = PyBytes_AsString(details); Py_DECREF(code); + if (c_code == -1 && PyErr_Occurred()) { + Py_DECREF(details); + return NULL; + } + c_message = PyBytes_AsString(details); Py_DECREF(details); + if (c_message == NULL) { + return NULL; + } call_error = grpc_call_start_write_status_old(self->c_call, c_code, c_message, (void *)tag); @@ -228,12 +233,12 @@ static PyMethodDef methods[] = { "Invoke this call."}, {"write", (PyCFunction)pygrpc_call_write, METH_VARARGS, "Write bytes to this call."}, - {"complete", (PyCFunction)pygrpc_call_complete, METH_VARARGS, + {"complete", (PyCFunction)pygrpc_call_complete, METH_O, "Complete writes to this call."}, {"accept", (PyCFunction)pygrpc_call_accept, METH_VARARGS, "Accept an RPC."}, {"premetadata", (PyCFunction)pygrpc_call_premetadata, METH_VARARGS, "Indicate the end of leading metadata in the response."}, - {"read", (PyCFunction)pygrpc_call_read, METH_VARARGS, + {"read", (PyCFunction)pygrpc_call_read, METH_O, "Read bytes from this call."}, {"status", (PyCFunction)pygrpc_call_status, METH_VARARGS, "Report this call's status."}, @@ -242,7 +247,7 @@ static PyMethodDef methods[] = { {NULL}}; PyTypeObject pygrpc_CallType = { - PyObject_HEAD_INIT(NULL)0, /*ob_size*/ + PyVarObject_HEAD_INIT(NULL, 0) "_grpc.Call", /*tp_name*/ sizeof(Call), /*tp_basicsize*/ 0, /*tp_itemsize*/ @@ -278,16 +283,16 @@ PyTypeObject pygrpc_CallType = { 0, /* tp_descr_set */ 0, /* tp_dictoffset */ (initproc)pygrpc_call_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; int pygrpc_add_call(PyObject *module) { - pygrpc_CallType.tp_new = PyType_GenericNew; if (PyType_Ready(&pygrpc_CallType) < 0) { - PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_CallType!"); return -1; } if (PyModule_AddObject(module, "Call", (PyObject *)&pygrpc_CallType) == -1) { - PyErr_SetString(PyExc_ImportError, "Couldn't add Call type to module!"); + return -1; } return 0; } diff --git a/src/python/src/grpc/_adapter/_channel.c b/src/python/src/grpc/_adapter/_channel.c index 6962722ed2..3ba943e4b2 100644 --- a/src/python/src/grpc/_adapter/_channel.c +++ b/src/python/src/grpc/_adapter/_channel.c @@ -38,9 +38,10 @@ static int pygrpc_channel_init(Channel *self, PyObject *args, PyObject *kwds) { const char *hostport; + static char *kwlist[] = {"hostport", NULL}; - if (!(PyArg_ParseTuple(args, "s", &hostport))) { - self->c_channel = NULL; + if (!(PyArg_ParseTupleAndKeywords(args, kwds, "s:Channel", kwlist, + &hostport))) { return -1; } @@ -56,7 +57,7 @@ static void pygrpc_channel_dealloc(Channel *self) { } PyTypeObject pygrpc_ChannelType = { - PyObject_HEAD_INIT(NULL)0, /*ob_size*/ + PyVarObject_HEAD_INIT(NULL, 0) "_grpc.Channel", /*tp_name*/ sizeof(Channel), /*tp_basicsize*/ 0, /*tp_itemsize*/ @@ -92,17 +93,16 @@ PyTypeObject pygrpc_ChannelType = { 0, /* tp_descr_set */ 0, /* tp_dictoffset */ (initproc)pygrpc_channel_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; int pygrpc_add_channel(PyObject *module) { - pygrpc_ChannelType.tp_new = PyType_GenericNew; if (PyType_Ready(&pygrpc_ChannelType) < 0) { - PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_ChannelType!"); return -1; } if (PyModule_AddObject(module, "Channel", (PyObject *)&pygrpc_ChannelType) == -1) { - PyErr_SetString(PyExc_ImportError, "Couldn't add Channel type to module!"); return -1; } return 0; diff --git a/src/python/src/grpc/_adapter/_completion_queue.c b/src/python/src/grpc/_adapter/_completion_queue.c index 1d593d0d14..b56ca1926e 100644 --- a/src/python/src/grpc/_adapter/_completion_queue.c +++ b/src/python/src/grpc/_adapter/_completion_queue.c @@ -70,7 +70,7 @@ static PyObject *metadata_event_kind; static PyObject *finish_event_kind; static PyObject *pygrpc_as_py_time(gpr_timespec *timespec) { - return Py_BuildValue("f", + return PyFloat_FromDouble( timespec->tv_sec + ((double)timespec->tv_nsec) / 1.0E9); } @@ -116,67 +116,82 @@ static PyObject *pygrpc_status_code(grpc_status_code c_status_code) { } static PyObject *pygrpc_stop_event_args(grpc_event *c_event) { - return Py_BuildValue("(OOOOOOO)", stop_event_kind, Py_None, Py_None, Py_None, - Py_None, Py_None, Py_None); + return PyTuple_Pack(7, stop_event_kind, Py_None, Py_None, Py_None, + Py_None, Py_None, Py_None); } static PyObject *pygrpc_write_event_args(grpc_event *c_event) { PyObject *write_accepted = c_event->data.write_accepted == GRPC_OP_OK ? Py_True : Py_False; - return Py_BuildValue("(OOOOOOO)", write_event_kind, (PyObject *)c_event->tag, - write_accepted, Py_None, Py_None, Py_None, Py_None); + return PyTuple_Pack(7, write_event_kind, (PyObject *)c_event->tag, + write_accepted, Py_None, Py_None, Py_None, Py_None); } static PyObject *pygrpc_complete_event_args(grpc_event *c_event) { PyObject *complete_accepted = c_event->data.finish_accepted == GRPC_OP_OK ? Py_True : Py_False; - return Py_BuildValue("(OOOOOOO)", complete_event_kind, - (PyObject *)c_event->tag, Py_None, complete_accepted, - Py_None, Py_None, Py_None); + return PyTuple_Pack(7, complete_event_kind, (PyObject *)c_event->tag, + Py_None, complete_accepted, Py_None, Py_None, Py_None); } static PyObject *pygrpc_service_event_args(grpc_event *c_event) { if (c_event->data.server_rpc_new.method == NULL) { - return Py_BuildValue("(OOOOOOO)", service_event_kind, c_event->tag, - Py_None, Py_None, Py_None, Py_None, Py_None); + return PyTuple_Pack(7, service_event_kind, c_event->tag, + Py_None, Py_None, Py_None, Py_None, Py_None); } else { - PyObject *method = PyBytes_FromString(c_event->data.server_rpc_new.method); - PyObject *host = PyBytes_FromString(c_event->data.server_rpc_new.host); - PyObject *service_deadline = + PyObject *method = NULL; + PyObject *host = NULL; + PyObject *service_deadline = NULL; + Call *call = NULL; + PyObject *service_acceptance = NULL; + PyObject *event_args = NULL; + + method = PyBytes_FromString(c_event->data.server_rpc_new.method); + if (method == NULL) { + goto error; + } + host = PyBytes_FromString(c_event->data.server_rpc_new.host); + if (host == NULL) { + goto error; + } + service_deadline = pygrpc_as_py_time(&c_event->data.server_rpc_new.deadline); - - Call *call; - PyObject *service_acceptance_args; - PyObject *service_acceptance; - PyObject *event_args; + if (service_deadline == NULL) { + goto error; + } call = PyObject_New(Call, &pygrpc_CallType); + if (call == NULL) { + goto error; + } call->c_call = c_event->call; - service_acceptance_args = - Py_BuildValue("(OOOO)", call, method, host, service_deadline); - Py_DECREF(call); - Py_DECREF(method); - Py_DECREF(host); - Py_DECREF(service_deadline); - service_acceptance = - PyObject_CallObject(service_acceptance_class, service_acceptance_args); - Py_DECREF(service_acceptance_args); + PyObject_CallFunctionObjArgs(service_acceptance_class, call, method, + host, service_deadline, NULL); + if (service_acceptance == NULL) { + goto error; + } + + event_args = PyTuple_Pack(7, service_event_kind, + (PyObject *)c_event->tag, Py_None, Py_None, + service_acceptance, Py_None, Py_None); - event_args = Py_BuildValue("(OOOOOOO)", service_event_kind, - (PyObject *)c_event->tag, Py_None, Py_None, - service_acceptance, Py_None, Py_None); Py_DECREF(service_acceptance); +error: + Py_XDECREF(call); + Py_XDECREF(method); + Py_XDECREF(host); + Py_XDECREF(service_deadline); + return event_args; } } static PyObject *pygrpc_read_event_args(grpc_event *c_event) { if (c_event->data.read == NULL) { - return Py_BuildValue("(OOOOOOO)", read_event_kind, - (PyObject *)c_event->tag, Py_None, Py_None, Py_None, - Py_None, Py_None); + return PyTuple_Pack(7, read_event_kind, (PyObject *)c_event->tag, + Py_None, Py_None, Py_None, Py_None, Py_None); } else { size_t length; size_t offset; @@ -198,9 +213,11 @@ static PyObject *pygrpc_read_event_args(grpc_event *c_event) { grpc_byte_buffer_reader_destroy(reader); bytes = PyBytes_FromStringAndSize(c_bytes, length); gpr_free(c_bytes); - event_args = - Py_BuildValue("(OOOOOOO)", read_event_kind, (PyObject *)c_event->tag, - Py_None, Py_None, Py_None, bytes, Py_None); + if (bytes == NULL) { + return NULL; + } + event_args = PyTuple_Pack(7, read_event_kind, (PyObject *)c_event->tag, + Py_None, Py_None, Py_None, bytes, Py_None); Py_DECREF(bytes); return event_args; } @@ -208,15 +225,13 @@ static PyObject *pygrpc_read_event_args(grpc_event *c_event) { static PyObject *pygrpc_metadata_event_args(grpc_event *c_event) { /* TODO(nathaniel): Actual transmission of metadata. */ - return Py_BuildValue("(OOOOOOO)", metadata_event_kind, - (PyObject *)c_event->tag, Py_None, Py_None, Py_None, - Py_None, Py_None); + return PyTuple_Pack(7, metadata_event_kind, (PyObject *)c_event->tag, + Py_None, Py_None, Py_None, Py_None, Py_None); } static PyObject *pygrpc_finished_event_args(grpc_event *c_event) { PyObject *code; PyObject *details; - PyObject *status_args; PyObject *status; PyObject *event_args; @@ -230,19 +245,26 @@ static PyObject *pygrpc_finished_event_args(grpc_event *c_event) { } else { details = PyBytes_FromString(c_event->data.finished.details); } - status_args = Py_BuildValue("(OO)", code, details); + if (details == NULL) { + return NULL; + } + status = PyObject_CallFunctionObjArgs(status_class, code, details, NULL); Py_DECREF(details); - status = PyObject_CallObject(status_class, status_args); - Py_DECREF(status_args); - event_args = - Py_BuildValue("(OOOOOOO)", finish_event_kind, (PyObject *)c_event->tag, - Py_None, Py_None, Py_None, Py_None, status); + if (status == NULL) { + return NULL; + } + event_args = PyTuple_Pack(7, finish_event_kind, (PyObject *)c_event->tag, + Py_None, Py_None, Py_None, Py_None, status); Py_DECREF(status); return event_args; } static int pygrpc_completion_queue_init(CompletionQueue *self, PyObject *args, PyObject *kwds) { + static char *kwlist[] = {NULL}; + if (!PyArg_ParseTupleAndKeywords(args, kwds, ":CompletionQueue", kwlist)) { + return -1; + } self->c_completion_queue = grpc_completion_queue_create(); return 0; } @@ -262,7 +284,7 @@ static PyObject *pygrpc_completion_queue_get(CompletionQueue *self, PyObject *event_args; PyObject *event; - if (!(PyArg_ParseTuple(args, "O", &deadline))) { + if (!(PyArg_ParseTuple(args, "O:get", &deadline))) { return NULL; } @@ -270,6 +292,9 @@ static PyObject *pygrpc_completion_queue_get(CompletionQueue *self, deadline_timespec = gpr_inf_future; } else { double_deadline = PyFloat_AsDouble(deadline); + if (PyErr_Occurred()) { + return NULL; + } deadline_timespec = gpr_time_from_nanos((long)(double_deadline * 1.0E9)); } @@ -339,7 +364,7 @@ static PyMethodDef methods[] = { {NULL}}; PyTypeObject pygrpc_CompletionQueueType = { - PyObject_HEAD_INIT(NULL)0, /*ob_size*/ + PyVarObject_HEAD_INIT(NULL, 0) "_gprc.CompletionQueue", /*tp_name*/ sizeof(CompletionQueue), /*tp_basicsize*/ 0, /*tp_itemsize*/ @@ -375,6 +400,8 @@ PyTypeObject pygrpc_CompletionQueueType = { 0, /* tp_descr_set */ 0, /* tp_dictoffset */ (initproc)pygrpc_completion_queue_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; static int pygrpc_get_status_codes(PyObject *datatypes_module) { @@ -503,7 +530,6 @@ int pygrpc_add_completion_queue(PyObject *module) { char *datatypes_module_path = "grpc._adapter._datatypes"; PyObject *datatypes_module = PyImport_ImportModule(datatypes_module_path); if (datatypes_module == NULL) { - PyErr_SetString(PyExc_ImportError, datatypes_module_path); return -1; } status_class = PyObject_GetAttrString(datatypes_module, "Status"); @@ -512,29 +538,21 @@ int pygrpc_add_completion_queue(PyObject *module) { event_class = PyObject_GetAttrString(datatypes_module, "Event"); if (status_class == NULL || service_acceptance_class == NULL || event_class == NULL) { - PyErr_SetString(PyExc_ImportError, "Missing classes in _datatypes module!"); return -1; } if (pygrpc_get_status_codes(datatypes_module) == -1) { - PyErr_SetString(PyExc_ImportError, "Status codes import broken!"); return -1; } if (pygrpc_get_event_kinds(event_class) == -1) { - PyErr_SetString(PyExc_ImportError, "Event kinds import broken!"); return -1; } Py_DECREF(datatypes_module); - pygrpc_CompletionQueueType.tp_new = PyType_GenericNew; if (PyType_Ready(&pygrpc_CompletionQueueType) < 0) { - PyErr_SetString(PyExc_RuntimeError, - "Error defining pygrpc_CompletionQueueType!"); return -1; } if (PyModule_AddObject(module, "CompletionQueue", (PyObject *)&pygrpc_CompletionQueueType) == -1) { - PyErr_SetString(PyExc_ImportError, - "Couldn't add CompletionQueue type to module!"); return -1; } return 0; diff --git a/src/python/src/grpc/_adapter/_server.c b/src/python/src/grpc/_adapter/_server.c index d4bf5fb8f6..ae7ae5b5d2 100644 --- a/src/python/src/grpc/_adapter/_server.c +++ b/src/python/src/grpc/_adapter/_server.c @@ -43,9 +43,11 @@ static int pygrpc_server_init(Server *self, PyObject *args, PyObject *kwds) { const PyObject *completion_queue; PyObject *server_credentials; - if (!(PyArg_ParseTuple(args, "O!O", &pygrpc_CompletionQueueType, - &completion_queue, &server_credentials))) { - self->c_server = NULL; + static char *kwlist[] = {"completion_queue", "server_credentials", NULL}; + + if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!O:Server", kwlist, + &pygrpc_CompletionQueueType, + &completion_queue, &server_credentials)) { return -1; } if (server_credentials == Py_None) { @@ -59,7 +61,9 @@ static int pygrpc_server_init(Server *self, PyObject *args, PyObject *kwds) { ((CompletionQueue *)completion_queue)->c_completion_queue, NULL); return 0; } else { - self->c_server = NULL; + PyErr_Format(PyExc_TypeError, + "server_credentials must be _grpc.ServerCredentials, not %s", + Py_TYPE(server_credentials)->tp_name); return -1; } } @@ -74,7 +78,9 @@ static void pygrpc_server_dealloc(Server *self) { static PyObject *pygrpc_server_add_http2_addr(Server *self, PyObject *args) { const char *addr; int port; - PyArg_ParseTuple(args, "s", &addr); + if (!PyArg_ParseTuple(args, "s:add_http2_addr", &addr)) { + return NULL; + } port = grpc_server_add_http2_port(self->c_server, addr); if (port == 0) { @@ -89,7 +95,9 @@ static PyObject *pygrpc_server_add_secure_http2_addr(Server *self, PyObject *args) { const char *addr; int port; - PyArg_ParseTuple(args, "s", &addr); + if (!PyArg_ParseTuple(args, "s:add_secure_http2_addr", &addr)) { + return NULL; + } port = grpc_server_add_secure_http2_port(self->c_server, addr); if (port == 0) { PyErr_SetString(PyExc_RuntimeError, "Couldn't add port to server!"); @@ -104,15 +112,10 @@ static PyObject *pygrpc_server_start(Server *self) { Py_RETURN_NONE; } -static const PyObject *pygrpc_server_service(Server *self, PyObject *args) { - const PyObject *tag; +static const PyObject *pygrpc_server_service(Server *self, PyObject *tag) { grpc_call_error call_error; const PyObject *result; - if (!(PyArg_ParseTuple(args, "O", &tag))) { - return NULL; - } - call_error = grpc_server_request_call_old(self->c_server, (void *)tag); result = pygrpc_translate_call_error(call_error); @@ -135,13 +138,13 @@ static PyMethodDef methods[] = { METH_VARARGS, "Add a secure HTTP2 address."}, {"start", (PyCFunction)pygrpc_server_start, METH_NOARGS, "Starts the server."}, - {"service", (PyCFunction)pygrpc_server_service, METH_VARARGS, + {"service", (PyCFunction)pygrpc_server_service, METH_O, "Services a call."}, {"stop", (PyCFunction)pygrpc_server_stop, METH_NOARGS, "Stops the server."}, {NULL}}; static PyTypeObject pygrpc_ServerType = { - PyObject_HEAD_INIT(NULL)0, /*ob_size*/ + PyVarObject_HEAD_INIT(NULL, 0) "_gprc.Server", /*tp_name*/ sizeof(Server), /*tp_basicsize*/ 0, /*tp_itemsize*/ @@ -177,17 +180,16 @@ static PyTypeObject pygrpc_ServerType = { 0, /* tp_descr_set */ 0, /* tp_dictoffset */ (initproc)pygrpc_server_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; int pygrpc_add_server(PyObject *module) { - pygrpc_ServerType.tp_new = PyType_GenericNew; if (PyType_Ready(&pygrpc_ServerType) < 0) { - PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_ServerType!"); return -1; } if (PyModule_AddObject(module, "Server", (PyObject *)&pygrpc_ServerType) == -1) { - PyErr_SetString(PyExc_ImportError, "Couldn't add Server type to module!"); return -1; } return 0; diff --git a/src/python/src/grpc/_adapter/_server_credentials.c b/src/python/src/grpc/_adapter/_server_credentials.c index ae85fd3eb7..06e6b94974 100644 --- a/src/python/src/grpc/_adapter/_server_credentials.c +++ b/src/python/src/grpc/_adapter/_server_credentials.c @@ -47,21 +47,20 @@ static int pygrpc_server_credentials_init(ServerCredentials *self, PyObject *iterator; int i; PyObject *pair; + static char *kwlist[] = {"root_credentials", "pair_sequence", NULL}; - if (!(PyArg_ParseTuple(args, "zO", &root_certificates, &pair_sequence))) { - self->c_server_credentials = NULL; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "zO:ServerCredentials", kwlist, + &root_certificates, &pair_sequence)) { return -1; } pair_count = PySequence_Length(pair_sequence); if (pair_count == -1) { - self->c_server_credentials = NULL; return -1; } iterator = PyObject_GetIter(pair_sequence); if (iterator == NULL) { - self->c_server_credentials = NULL; return -1; } pairs = gpr_malloc(pair_count * sizeof(grpc_ssl_pem_key_cert_pair)); @@ -72,8 +71,8 @@ static int pygrpc_server_credentials_init(ServerCredentials *self, error = 1; break; } - if (!(PyArg_ParseTuple(pair, "ss", &pairs[i].private_key, - &pairs[i].cert_chain))) { + if (!PyArg_ParseTuple(pair, "ss", &pairs[i].private_key, + &pairs[i].cert_chain)) { error = 1; Py_DECREF(pair); break; @@ -83,7 +82,6 @@ static int pygrpc_server_credentials_init(ServerCredentials *self, Py_DECREF(iterator); if (error) { - self->c_server_credentials = NULL; gpr_free(pairs); return -1; } else { @@ -102,7 +100,7 @@ static void pygrpc_server_credentials_dealloc(ServerCredentials *self) { } PyTypeObject pygrpc_ServerCredentialsType = { - PyObject_HEAD_INIT(NULL)0, /*ob_size*/ + PyVarObject_HEAD_INIT(NULL, 0) "_grpc.ServerCredencials", /*tp_name*/ sizeof(ServerCredentials), /*tp_basicsize*/ 0, /*tp_itemsize*/ @@ -138,19 +136,16 @@ PyTypeObject pygrpc_ServerCredentialsType = { 0, /* tp_descr_set */ 0, /* tp_dictoffset */ (initproc)pygrpc_server_credentials_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; int pygrpc_add_server_credentials(PyObject *module) { - pygrpc_ServerCredentialsType.tp_new = PyType_GenericNew; if (PyType_Ready(&pygrpc_ServerCredentialsType) < 0) { - PyErr_SetString(PyExc_RuntimeError, - "Error defining pygrpc_ServerCredentialsType!"); return -1; } if (PyModule_AddObject(module, "ServerCredentials", (PyObject *)&pygrpc_ServerCredentialsType) == -1) { - PyErr_SetString(PyExc_ImportError, - "Couldn't add ServerCredentials type to module!"); return -1; } return 0; -- cgit v1.2.3