Antoine Pitrou <[email protected]> added the comment: Some notes about the patch:
- NEWS blurbs generally appear antichronologically, that is newest first :-) - you don't have to mention that "The 3.2 changes mentioned above are included in 2.7" - the part of the PyArg_ParseTupleAndKeywords string after the semi-colon is supposed to be a custom error message, which "[ident string [, logoption [, facility]]]" doesn't look like; how about simply using a colon and putting the function name? - you should check PyList_Size()'s return value for error (sys.argv could have been overriden to something else than a list) - similarly, scriptobj is not guaranteed to be a string object, so you have to check for that too - to check for string length > 0, it looks more natural to check either that script[0] != 0, or that PyString_GET_SIZE(...) > 0 (rather than calling PyObject_IsTrue()) - why do you Py_INCREF() the "openargs" tuple? It looks like a reference leak - if PyTuple_New(0) fails (which I admit is highly unlikely), you should free all other resources and bail out - when manually calling syslog_openlog(), you should check the return value and either Py_DECREF it (if non-NULL) or bail out ---------- nosy: +pitrou _______________________________________ Python tracker <[email protected]> <http://bugs.python.org/issue8451> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
