readline module cleanup

fixed indention to tabs
use Py_RETURN_NONE macro
added more error checks to on_completion_display_matches_hook

open question: Does PyList_SetItem(l, i, o) steal a reference to o in the case of an error?

diff --git a/Modules/readline.c b/Modules/readline.c
index 85bbafdf4ed88b8d4a9a9ea818d4d8454c80e37f..90904ab434faac37f7ade09030df7f285bb2d695 100644 (file)
--- a/Modules/readline.c
+++ b/Modules/readline.c
@@ -59,8 +59,7 @@ parse_and_bind(PyObject *self, PyObject *args)
        strcpy(copy, s);
        rl_parse_and_bind(copy);
        free(copy); /* Free the copy */
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_parse_and_bind,
@@ -79,8 +78,7 @@ read_init_file(PyObject *self, PyObject *args)
        errno = rl_read_init_file(s);
        if (errno)
                return PyErr_SetFromErrno(PyExc_IOError);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_read_init_file,
@@ -100,8 +98,7 @@ read_history_file(PyObject *self, PyObject *args)
        errno = read_history(s);
        if (errno)
                return PyErr_SetFromErrno(PyExc_IOError);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 static int _history_length = -1; /* do not truncate history by default */
@@ -124,8 +121,7 @@ write_history_file(PyObject *self, PyObject *args)
                history_truncate_file(s, _history_length);
        if (errno)
                return PyErr_SetFromErrno(PyExc_IOError);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_write_history_file,
@@ -143,8 +139,7 @@ set_history_length(PyObject *self, PyObject *args)
        if (!PyArg_ParseTuple(args, "i:set_history_length", &length))
                return NULL;
        _history_length = length;
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(set_history_length_doc,
@@ -195,8 +190,7 @@ set_hook(const char *funcname, PyObject **hook_var, PyObject *args)
                PyErr_SetString(PyExc_TypeError, buf);
                return NULL;
        }
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 
@@ -218,7 +212,7 @@ set_completion_display_matches_hook(PyObject *self, PyObject *args)
        /* We cannot set this hook globally, since it replaces the
           default completion display. */
        rl_completion_display_matches_hook =
-         completion_display_matches_hook ? 
+               completion_display_matches_hook ?
                (rl_compdisp_func_t *)on_completion_display_matches_hook : 0;
 #endif
        return result;
@@ -325,8 +319,7 @@ set_completer_delims(PyObject *self, PyObject *args)
        }
        free((void*)rl_completer_word_break_characters);
        rl_completer_word_break_characters = strdup(break_chars);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_set_completer_delims,
@@ -336,32 +329,31 @@ set the readline word delimiters for tab-completion");
 static PyObject *
 py_remove_history(PyObject *self, PyObject *args)
 {
-        int entry_number;
-        HIST_ENTRY *entry;
-
-        if (!PyArg_ParseTuple(args, "i:remove_history", &entry_number))
-                return NULL;
-        if (entry_number < 0) {
-                PyErr_SetString(PyExc_ValueError,
-                                "History index cannot be negative");
-                return NULL;
-        }
-        entry = remove_history(entry_number);
-        if (!entry) {
-                PyErr_Format(PyExc_ValueError,
-                             "No history item at position %d",
-                             entry_number);
-                return NULL;
-        }
-        /* free memory allocated for the history entry */
-        if (entry->line)
-                free(entry->line);
-        if (entry->data)
-                free(entry->data);
-        free(entry);
-
-        Py_INCREF(Py_None);
-        return Py_None;
+       int entry_number;
+       HIST_ENTRY *entry;
+
+       if (!PyArg_ParseTuple(args, "i:remove_history", &entry_number))
+               return NULL;
+       if (entry_number < 0) {
+               PyErr_SetString(PyExc_ValueError,
+                               "History index cannot be negative");
+               return NULL;
+       }
+       entry = remove_history(entry_number);
+       if (!entry) {
+               PyErr_Format(PyExc_ValueError,
+                            "No history item at position %d",
+                             entry_number);
+               return NULL;
+       }
+       /* free memory allocated for the history entry */
+       if (entry->line)
+               free(entry->line);
+       if (entry->data)
+               free(entry->data);
+       free(entry);
+
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_remove_history,
@@ -371,34 +363,34 @@ remove history item given by its position");
 static PyObject *
 py_replace_history(PyObject *self, PyObject *args)
 {
-        int entry_number;
-        char *line;
-        HIST_ENTRY *old_entry;
-
-        if (!PyArg_ParseTuple(args, "is:replace_history", &entry_number, &line)) {
-                return NULL;
-        }
-        if (entry_number < 0) {
-                PyErr_SetString(PyExc_ValueError,
-                                "History index cannot be negative");
-                return NULL;
-        }
-        old_entry = replace_history_entry(entry_number, line, (void *)NULL);
-        if (!old_entry) {
-                PyErr_Format(PyExc_ValueError,
-                             "No history item at position %d",
-                             entry_number);
-                return NULL;
-        }
-        /* free memory allocated for the old history entry */
-        if (old_entry->line)
-            free(old_entry->line);
-        if (old_entry->data)
-            free(old_entry->data);
-        free(old_entry);
-
-        Py_INCREF(Py_None);
-        return Py_None;
+       int entry_number;
+       char *line;
+       HIST_ENTRY *old_entry;
+
+       if (!PyArg_ParseTuple(args, "is:replace_history", &entry_number,
+                             &line)) {
+               return NULL;
+       }
+       if (entry_number < 0) {
+               PyErr_SetString(PyExc_ValueError,
+                               "History index cannot be negative");
+               return NULL;
+       }
+       old_entry = replace_history_entry(entry_number, line, (void *)NULL);
+       if (!old_entry) {
+               PyErr_Format(PyExc_ValueError,
+                            "No history item at position %d",
+                            entry_number);
+               return NULL;
+       }
+       /* free memory allocated for the old history entry */
+       if (old_entry->line)
+           free(old_entry->line);
+       if (old_entry->data)
+           free(old_entry->data);
+       free(old_entry);
+
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_replace_history,
@@ -416,8 +408,7 @@ py_add_history(PyObject *self, PyObject *args)
                return NULL;
        }
        add_history(line);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_add_history,
@@ -458,8 +449,7 @@ static PyObject *
 get_completer(PyObject *self, PyObject *noargs)
 {
        if (completer == NULL) {
-               Py_INCREF(Py_None);
-               return Py_None;
+               Py_RETURN_NONE;
        }
        Py_INCREF(completer);
        return completer;
@@ -483,8 +473,7 @@ get_history_item(PyObject *self, PyObject *args)
        if ((hist_ent = history_get(idx)))
                return PyString_FromString(hist_ent->line);
        else {
-               Py_INCREF(Py_None);
-               return Py_None;
+               Py_RETURN_NONE;
        }
 }
 
@@ -530,8 +519,7 @@ static PyObject *
 py_clear_history(PyObject *self, PyObject *noarg)
 {
        clear_history();
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_clear_history,
@@ -549,8 +537,7 @@ insert_text(PyObject *self, PyObject *args)
        if (!PyArg_ParseTuple(args, "s:insert_text", &s))
                return NULL;
        rl_insert_text(s);
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_insert_text,
@@ -564,8 +551,7 @@ static PyObject *
 redisplay(PyObject *self, PyObject *noarg)
 {
        rl_redisplay();
-       Py_INCREF(Py_None);
-       return Py_None;
+       Py_RETURN_NONE;
 }
 
 PyDoc_STRVAR(doc_redisplay,
@@ -591,9 +577,9 @@ static struct PyMethodDef readline_methods[] =
         METH_VARARGS, doc_get_history_item},
        {"get_current_history_length", (PyCFunction)get_current_history_length,
         METH_NOARGS, doc_get_current_history_length},
-       {"set_history_length", set_history_length,
+       {"set_history_length", set_history_length,
         METH_VARARGS, set_history_length_doc},
-       {"get_history_length", get_history_length,
+       {"get_history_length", get_history_length,
         METH_NOARGS, get_history_length_doc},
        {"set_completer", set_completer, METH_VARARGS, doc_set_completer},
        {"get_completer", get_completer, METH_NOARGS, doc_get_completer},
@@ -605,8 +591,8 @@ static struct PyMethodDef readline_methods[] =
        {"set_completer_delims", set_completer_delims,
         METH_VARARGS, doc_set_completer_delims},
        {"add_history", py_add_history, METH_VARARGS, doc_add_history},
-        {"remove_history_item", py_remove_history, METH_VARARGS, doc_remove_history},
-        {"replace_history_item", py_replace_history, METH_VARARGS, doc_replace_history},
+       {"remove_history_item", py_remove_history, METH_VARARGS, doc_remove_history},
+       {"replace_history_item", py_replace_history, METH_VARARGS, doc_replace_history},
        {"get_completer_delims", get_completer_delims,
         METH_NOARGS, doc_get_completer_delims},
 
@@ -633,7 +619,7 @@ on_hook(PyObject *func)
        int result = 0;
        if (func != NULL) {
                PyObject *r;
-#ifdef WITH_THREAD           
+#ifdef WITH_THREAD
                PyGILState_STATE gilstate = PyGILState_Ensure();
 #endif
                r = PyObject_CallFunction(func, NULL);
@@ -652,7 +638,7 @@ on_hook(PyObject *func)
                PyErr_Clear();
                Py_XDECREF(r);
          done:
-#ifdef WITH_THREAD           
+#ifdef WITH_THREAD
                PyGILState_Release(gilstate);
 #endif
                return result;
@@ -682,34 +668,39 @@ on_completion_display_matches_hook(char **matches,
                                   int num_matches, int max_length)
 {
        int i;
-       PyObject *m, *s;
-       PyObject *r;
-#ifdef WITH_THREAD           
+       PyObject *m=NULL, *s=NULL, *r=NULL;
+#ifdef WITH_THREAD
        PyGILState_STATE gilstate = PyGILState_Ensure();
 #endif
        m = PyList_New(num_matches);
+       if (m == NULL)
+               goto error;
        for (i = 0; i < num_matches; i++) {
                s = PyString_FromString(matches[i+1]);
-               PyList_SetItem(m, i, s);
+               if (s == NULL)
+                       goto error;
+               if (PyList_SetItem(m, i, s) == -1)
+                       goto error;
        }
 
        r = PyObject_CallFunction(completion_display_matches_hook,
                                  "sOi", matches[0], m, max_length);
 
-       Py_DECREF(m);
+       Py_DECREF(m), m=NULL;
        
        if (r == NULL ||
            (r != Py_None && PyInt_AsLong(r) == -1 && PyErr_Occurred())) {
                goto error;
        }
+       Py_XDECREF(r), r=NULL;
 
-       Py_DECREF(r);
-       goto done;
-  error:
-       PyErr_Clear();
-       Py_XDECREF(r);
-  done:
-#ifdef WITH_THREAD           
+       if (0) {
+       error:
+               PyErr_Clear();
+               Py_XDECREF(m);
+               Py_XDECREF(r);
+       }
+#ifdef WITH_THREAD
        PyGILState_Release(gilstate);
 #endif
 }