From: Zbigniew Jędrzejewski-Szmek Date: Fri, 22 Feb 2013 12:33:06 +0000 (+0100) Subject: python-systemd: check all errors and use automatic cleanup X-Git-Tag: v198~140^2~2 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=6a6633a16a510b40c6ad30cd0858699619384a44 python-systemd: check all errors and use automatic cleanup __REALTIME_TIMESTAMP and __MONOTONIC_TIMESTAMP return ints. It doesn't make sense to convert to string, just to convert back to a number later on. Also try to follow systemd rules for indentation. --- diff --git a/Makefile.am b/Makefile.am index 4ff0cff8b..f1c2ce05b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3404,7 +3404,9 @@ _journal_la_LIBADD = \ id128_la_SOURCES = \ src/python-systemd/id128.c \ - src/python-systemd/id128-constants.h + src/python-systemd/id128-constants.h \ + src/python-systemd/pyutil.c \ + src/python-systemd/pyutil.h id128_la_CFLAGS = \ $(AM_CFLAGS) \ @@ -3423,7 +3425,9 @@ id128_la_LIBADD = \ libsystemd-id128.la _reader_la_SOURCES = \ - src/python-systemd/_reader.c + src/python-systemd/_reader.c \ + src/python-systemd/pyutil.c \ + src/python-systemd/pyutil.h _reader_la_CFLAGS = \ $(AM_CFLAGS) \ @@ -3439,7 +3443,8 @@ _reader_la_LDFLAGS = \ _reader_la_LIBADD = \ $(PYTHON_LIBS) \ libsystemd-journal.la \ - libsystemd-id128.la + libsystemd-id128.la \ + libsystemd-shared.la dist_pkgpyexec_PYTHON = \ src/python-systemd/journal.py \ @@ -3447,7 +3452,7 @@ dist_pkgpyexec_PYTHON = \ src/python-systemd/id128-constants.h: src/systemd/sd-messages.h Makefile $(AM_V_at)$(MKDIR_P) $(dir $@) - $(AM_V_GEN)$(SED) -n -r 's/,//g; s/#define (SD_MESSAGE_[A-Z0-9_]+)\s.*/add_id(m, "\1", \1);/p' <$< >$@ + $(AM_V_GEN)$(SED) -n -r 's/,//g; s/#define (SD_MESSAGE_[A-Z0-9_]+)\s.*/add_id(m, "\1", \1) JOINER/p' <$< >$@ BUILT_SOURCES += \ src/python-systemd/id128-constants.h diff --git a/src/python-systemd/_reader.c b/src/python-systemd/_reader.c index 207b9e76c..9262c89e4 100644 --- a/src/python-systemd/_reader.c +++ b/src/python-systemd/_reader.c @@ -18,11 +18,17 @@ You should have received a copy of the GNU Lesser General Public License along with systemd; If not, see . ***/ -#include #include #include #include +#include + +#include + +#include "pyutil.h" +#include "macro.h" +#include "util.h" #if PY_MAJOR_VERSION >=3 # define unicode_FromStringAndSize PyUnicode_FromStringAndSize @@ -107,10 +113,9 @@ static PyObject* Journal_get_next(Journal *self, PyObject *args) PyObject *dict; const void *msg; size_t msg_len; - const char *delim_ptr; - PyObject *key, *value, *cur_value, *tmp_list; + int64_t skip = 1LL; + int r; - int64_t skip = 1LL, r = -EINVAL; if (!PyArg_ParseTuple(args, "|L", &skip)) return NULL; @@ -128,6 +133,8 @@ static PyObject* Journal_get_next(Journal *self, PyObject *args) r = sd_journal_next_skip(self->j, skip); else if (skip < -1LL) r = sd_journal_previous_skip(self->j, -skip); + else + assert_not_reached("should not be here"); Py_END_ALLOW_THREADS set_error(r, NULL, NULL); @@ -137,70 +144,128 @@ static PyObject* Journal_get_next(Journal *self, PyObject *args) return PyDict_New(); dict = PyDict_New(); + if (!dict) + return NULL; SD_JOURNAL_FOREACH_DATA(self->j, msg, msg_len) { + PyObject _cleanup_Py_DECREF_ *key = NULL, *value = NULL; + const char *delim_ptr; + delim_ptr = memchr(msg, '=', msg_len); + if (!delim_ptr) { + PyErr_SetString(PyExc_OSError, + "journal gave us a field without '='"); + goto error; + } + key = unicode_FromStringAndSize(msg, delim_ptr - (const char*) msg); - value = PyBytes_FromStringAndSize(delim_ptr + 1, (const char*) msg + msg_len - (delim_ptr + 1) ); + if (!key) + goto error; + + value = PyBytes_FromStringAndSize( + delim_ptr + 1, + (const char*) msg + msg_len - (delim_ptr + 1) ); + if (!value) + goto error; + if (PyDict_Contains(dict, key)) { - cur_value = PyDict_GetItem(dict, key); + PyObject *cur_value = PyDict_GetItem(dict, key); + if (PyList_CheckExact(cur_value)) { - PyList_Append(cur_value, value); - }else{ - tmp_list = PyList_New(0); - PyList_Append(tmp_list, cur_value); - PyList_Append(tmp_list, value); + r = PyList_Append(cur_value, value); + if (r < 0) + goto error; + } else { + PyObject _cleanup_Py_DECREF_ *tmp_list = PyList_New(0); + if (!tmp_list) + goto error; + + r = PyList_Append(tmp_list, cur_value); + if (r < 0) + goto error; + + r = PyList_Append(tmp_list, value); + if (r < 0) + goto error; + PyDict_SetItem(dict, key, tmp_list); - Py_DECREF(tmp_list); + if (r < 0) + goto error; } - }else{ - PyDict_SetItem(dict, key, value); + } else { + r = PyDict_SetItem(dict, key, value); + if (r < 0) + goto error; } - Py_DECREF(key); - Py_DECREF(value); } { + PyObject _cleanup_Py_DECREF_ *key = NULL, *value = NULL; uint64_t realtime; - if (sd_journal_get_realtime_usec(self->j, &realtime) == 0) { - char realtime_str[20]; - sprintf(realtime_str, "%llu", (long long unsigned) realtime); - key = unicode_FromString("__REALTIME_TIMESTAMP"); - value = PyBytes_FromString(realtime_str); - PyDict_SetItem(dict, key, value); - Py_DECREF(key); - Py_DECREF(value); - } + + r = sd_journal_get_realtime_usec(self->j, &realtime); + if (set_error(r, NULL, NULL)) + goto error; + + key = unicode_FromString("__REALTIME_TIMESTAMP"); + if (!key) + goto error; + + assert_cc(sizeof(unsigned long long) == sizeof(realtime)); + value = PyLong_FromUnsignedLongLong(realtime); + if (!value) + goto error; + + if (PyDict_SetItem(dict, key, value)) + goto error; } { + PyObject _cleanup_Py_DECREF_ *key = NULL, *value = NULL; sd_id128_t sd_id; uint64_t monotonic; - if (sd_journal_get_monotonic_usec(self->j, &monotonic, &sd_id) == 0) { - char monotonic_str[20]; - sprintf(monotonic_str, "%llu", (long long unsigned) monotonic); - key = unicode_FromString("__MONOTONIC_TIMESTAMP"); - value = PyBytes_FromString(monotonic_str); - - PyDict_SetItem(dict, key, value); - Py_DECREF(key); - Py_DECREF(value); - } + + r = sd_journal_get_monotonic_usec(self->j, &monotonic, &sd_id); + if (set_error(r, NULL, NULL)) + goto error; + + key = unicode_FromString("__MONOTONIC_TIMESTAMP"); + if (!key) + goto error; + + assert_cc(sizeof(unsigned long long) == sizeof(monotonic)); + value = PyLong_FromUnsignedLongLong(monotonic); + if (!value) + goto error; + + if (PyDict_SetItem(dict, key, value)) + goto error; } { - char *cursor; - if (sd_journal_get_cursor(self->j, &cursor) > 0) { //Should return 0... - key = unicode_FromString("__CURSOR"); - value = PyBytes_FromString(cursor); - PyDict_SetItem(dict, key, value); - free(cursor); - Py_DECREF(key); - Py_DECREF(value); - } + PyObject _cleanup_Py_DECREF_ *key = NULL, *value = NULL; + char _cleanup_free_ *cursor = NULL; + + r = sd_journal_get_cursor(self->j, &cursor); + if (set_error(r, NULL, NULL)) + goto error; + + key = unicode_FromString("__CURSOR"); + if (!key) + goto error; + + value = PyBytes_FromString(cursor); + if (!value) + goto error; + + if (PyDict_SetItem(dict, key, value)) + goto error; } return dict; +error: + Py_DECREF(dict); + return NULL; } PyDoc_STRVAR(Journal_get_previous__doc__, @@ -451,7 +516,7 @@ static PyObject* Journal_iternext(PyObject *self) dict_size = PyDict_Size(dict); if ((int64_t) dict_size > 0LL) { return dict; - }else{ + } else { Py_DECREF(dict); PyErr_SetNone(PyExc_StopIteration); return NULL; @@ -486,7 +551,9 @@ static PyObject* Journal_query_unique(Journal *self, PyObject *args) const char *delim_ptr; delim_ptr = memchr(uniq, '=', uniq_len); - value = PyBytes_FromStringAndSize(delim_ptr + 1, (const char*) uniq + uniq_len - (delim_ptr + 1)); + value = PyBytes_FromStringAndSize( + delim_ptr + 1, + (const char*) uniq + uniq_len - (delim_ptr + 1)); PySet_Add(value_set, value); Py_DECREF(value); } @@ -495,7 +562,7 @@ static PyObject* Journal_query_unique(Journal *self, PyObject *args) } PyDoc_STRVAR(data_threshold__doc__, - "Threshold for field size truncation.\n\n" + "Threshold for field size truncation in bytes.\n\n" "Fields longer than this will be truncated to the threshold size.\n" "Defaults to 64Kb."); @@ -515,7 +582,7 @@ static int Journal_set_data_threshold(Journal *self, PyObject *value, void *clos { int r; if (value == NULL) { - PyErr_SetString(PyExc_TypeError, "Cannot delete data threshold"); + PyErr_SetString(PyExc_AttributeError, "Cannot delete data threshold"); return -1; } if (!long_Check(value)){ @@ -528,78 +595,67 @@ static int Journal_set_data_threshold(Journal *self, PyObject *value, void *clos static PyGetSetDef Journal_getseters[] = { {(char*) "data_threshold", - (getter)Journal_get_data_threshold, - (setter)Journal_set_data_threshold, + (getter) Journal_get_data_threshold, + (setter) Journal_set_data_threshold, (char*) data_threshold__doc__, NULL}, {NULL} }; static PyMethodDef Journal_methods[] = { - {"get_next", (PyCFunction)Journal_get_next, METH_VARARGS, - Journal_get_next__doc__}, - {"get_previous", (PyCFunction)Journal_get_previous, METH_VARARGS, - Journal_get_previous__doc__}, - {"add_match", (PyCFunction)Journal_add_match, METH_VARARGS|METH_KEYWORDS, - Journal_add_match__doc__}, - {"add_disjunction", (PyCFunction)Journal_add_disjunction, METH_NOARGS, - Journal_add_disjunction__doc__}, - {"flush_matches", (PyCFunction)Journal_flush_matches, METH_NOARGS, - Journal_flush_matches__doc__}, - {"seek", (PyCFunction)Journal_seek, METH_VARARGS | METH_KEYWORDS, - Journal_seek__doc__}, - {"seek_realtime", (PyCFunction)Journal_seek_realtime, METH_VARARGS, - Journal_seek_realtime__doc__}, - {"seek_monotonic", (PyCFunction)Journal_seek_monotonic, METH_VARARGS, - Journal_seek_monotonic__doc__}, - {"wait", (PyCFunction)Journal_wait, METH_VARARGS, - Journal_wait__doc__}, - {"seek_cursor", (PyCFunction)Journal_seek_cursor, METH_VARARGS, - Journal_seek_cursor__doc__}, - {"query_unique", (PyCFunction)Journal_query_unique, METH_VARARGS, - Journal_query_unique__doc__}, + {"get_next", (PyCFunction) Journal_get_next, METH_VARARGS, Journal_get_next__doc__}, + {"get_previous", (PyCFunction) Journal_get_previous, METH_VARARGS, Journal_get_previous__doc__}, + {"add_match", (PyCFunction) Journal_add_match, METH_VARARGS|METH_KEYWORDS, Journal_add_match__doc__}, + {"add_disjunction", (PyCFunction) Journal_add_disjunction, METH_NOARGS, Journal_add_disjunction__doc__}, + {"flush_matches", (PyCFunction) Journal_flush_matches, METH_NOARGS, Journal_flush_matches__doc__}, + {"seek", (PyCFunction) Journal_seek, METH_VARARGS | METH_KEYWORDS, Journal_seek__doc__}, + {"seek_realtime", (PyCFunction) Journal_seek_realtime, METH_VARARGS, Journal_seek_realtime__doc__}, + {"seek_monotonic", (PyCFunction) Journal_seek_monotonic, METH_VARARGS, Journal_seek_monotonic__doc__}, + {"wait", (PyCFunction) Journal_wait, METH_VARARGS, Journal_wait__doc__}, + {"seek_cursor", (PyCFunction) Journal_seek_cursor, METH_VARARGS, Journal_seek_cursor__doc__}, + {"query_unique", (PyCFunction) Journal_query_unique, METH_VARARGS, Journal_query_unique__doc__}, {NULL} /* Sentinel */ }; static PyTypeObject JournalType = { PyVarObject_HEAD_INIT(NULL, 0) - "_reader._Journal", /*tp_name*/ - sizeof(Journal), /*tp_basicsize*/ - 0, /*tp_itemsize*/ - (destructor)Journal_dealloc, /*tp_dealloc*/ - 0, /*tp_print*/ - 0, /*tp_getattr*/ - 0, /*tp_setattr*/ - 0, /*tp_compare*/ - 0, /*tp_repr*/ - 0, /*tp_as_number*/ - 0, /*tp_as_sequence*/ - 0, /*tp_as_mapping*/ - 0, /*tp_hash */ - 0, /*tp_call*/ - 0, /*tp_str*/ - 0, /*tp_getattro*/ - 0, /*tp_setattro*/ - 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,/*tp_flags*/ - Journal__doc__, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - Journal_iter, /* tp_iter */ - Journal_iternext, /* tp_iternext */ - Journal_methods, /* tp_methods */ - 0, /* tp_members */ - Journal_getseters, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - (initproc)Journal_init, /* tp_init */ - 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + "_reader._Journal", /*tp_name*/ + sizeof(Journal), /*tp_basicsize*/ + 0, /*tp_itemsize*/ + (destructor)Journal_dealloc, /*tp_dealloc*/ + 0, /*tp_print*/ + 0, /*tp_getattr*/ + 0, /*tp_setattr*/ + 0, /*tp_compare*/ + 0, /*tp_repr*/ + 0, /*tp_as_number*/ + 0, /*tp_as_sequence*/ + 0, /*tp_as_mapping*/ + 0, /*tp_hash */ + 0, /*tp_call*/ + 0, /*tp_str*/ + 0, /*tp_getattro*/ + 0, /*tp_setattro*/ + 0, /*tp_as_buffer*/ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/ + Journal__doc__, /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + Journal_iter, /* tp_iter */ + Journal_iternext, /* tp_iternext */ + Journal_methods, /* tp_methods */ + 0, /* tp_members */ + Journal_getseters, /* tp_getset */ + 0, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + (initproc) Journal_init, /* tp_init */ + 0, /* tp_alloc */ + PyType_GenericNew, /* tp_new */ }; #define SUMMARY \ @@ -647,13 +703,18 @@ init_reader(void) #endif Py_INCREF(&JournalType); - PyModule_AddObject(m, "_Journal", (PyObject *)&JournalType); - PyModule_AddIntConstant(m, "NOP", SD_JOURNAL_NOP); - PyModule_AddIntConstant(m, "APPEND", SD_JOURNAL_APPEND); - PyModule_AddIntConstant(m, "INVALIDATE", SD_JOURNAL_INVALIDATE); - PyModule_AddIntConstant(m, "LOCAL_ONLY", SD_JOURNAL_LOCAL_ONLY); - PyModule_AddIntConstant(m, "RUNTIME_ONLY", SD_JOURNAL_RUNTIME_ONLY); - PyModule_AddIntConstant(m, "SYSTEM_ONLY", SD_JOURNAL_SYSTEM_ONLY); + if (PyModule_AddObject(m, "_Journal", (PyObject *) &JournalType) || + PyModule_AddIntConstant(m, "NOP", SD_JOURNAL_NOP) || + PyModule_AddIntConstant(m, "APPEND", SD_JOURNAL_APPEND) || + PyModule_AddIntConstant(m, "INVALIDATE", SD_JOURNAL_INVALIDATE) || + PyModule_AddIntConstant(m, "LOCAL_ONLY", SD_JOURNAL_LOCAL_ONLY) || + PyModule_AddIntConstant(m, "RUNTIME_ONLY", SD_JOURNAL_RUNTIME_ONLY) || + PyModule_AddIntConstant(m, "SYSTEM_ONLY", SD_JOURNAL_SYSTEM_ONLY)) { +#if PY_MAJOR_VERSION >= 3 + Py_DECREF(m); + return NULL; +#endif + } #if PY_MAJOR_VERSION >= 3 return m; diff --git a/src/python-systemd/id128.c b/src/python-systemd/id128.c index 42f247d10..a6711a5bd 100644 --- a/src/python-systemd/id128.c +++ b/src/python-systemd/id128.c @@ -19,18 +19,13 @@ along with systemd; If not, see . ***/ +#include + #include #include -#define _cleanup_Py_DECREF_ __attribute__((cleanup(cleanup_Py_DECREFp))) - -static void cleanup_Py_DECREFp(PyObject **p) { - if (!*p) - return; - - Py_DECREF(*p); -} +#include "pyutil.h" PyDoc_STRVAR(module__doc__, "Python interface to the libsystemd-id128 library.\n\n" @@ -127,7 +122,10 @@ PyMODINIT_FUNC initid128(void) { if (m == NULL) return; + /* a series of lines like 'add_id() ;' follow */ +#define JOINER ; #include "id128-constants.h" +#undef JOINER } #else @@ -147,7 +145,14 @@ PyMODINIT_FUNC PyInit_id128(void) { if (m == NULL) return NULL; + if ( /* a series of lines like 'add_id() ||' follow */ +#define JOINER || #include "id128-constants.h" +#undef JOINER + false) { + Py_DECREF(m); + return NULL; + } return m; } diff --git a/src/python-systemd/journal.py b/src/python-systemd/journal.py index 4d71564c6..d94934cfa 100644 --- a/src/python-systemd/journal.py +++ b/src/python-systemd/journal.py @@ -19,6 +19,8 @@ # You should have received a copy of the GNU Lesser General Public License # along with systemd; If not, see . +from __future__ import division + import sys as _sys import datetime as _datetime import functools as _functools @@ -36,8 +38,8 @@ from ._reader import (_Journal, NOP, APPEND, INVALIDATE, LOCAL_ONLY, RUNTIME_ONLY, SYSTEM_ONLY) from . import id128 as _id128 -_MONOTONIC_CONVERTER = lambda x: _datetime.timedelta(microseconds=float(x)) -_REALTIME_CONVERTER = lambda x: _datetime.datetime.fromtimestamp(float(x)/1E6) +_MONOTONIC_CONVERTER = lambda x: _datetime.timedelta(microseconds=x) +_REALTIME_CONVERTER = lambda x: _datetime.datetime.fromtimestamp(x / 1E6) DEFAULT_CONVERTERS = { 'MESSAGE_ID': _uuid.UUID, '_MACHINE_ID': _uuid.UUID, diff --git a/src/python-systemd/pyutil.c b/src/python-systemd/pyutil.c new file mode 100644 index 000000000..79065a11c --- /dev/null +++ b/src/python-systemd/pyutil.c @@ -0,0 +1,30 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2013 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include +#include "pyutil.h" + +void cleanup_Py_DECREFp(PyObject **p) { + if (!*p) + return; + + Py_DECREF(*p); +} diff --git a/src/python-systemd/pyutil.h b/src/python-systemd/pyutil.h new file mode 100644 index 000000000..3b7bc580d --- /dev/null +++ b/src/python-systemd/pyutil.h @@ -0,0 +1,29 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2013 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#ifndef Py_TYPE +/* avoid duplication warnings from errors in Python 2.7 headers */ +# include +#endif + +void cleanup_Py_DECREFp(PyObject **p); + +#define _cleanup_Py_DECREF_ __attribute__((cleanup(cleanup_Py_DECREFp)))