>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
Lots of nits on this one, mostly I think because it was converted from
C? Anything, nothing very major, just lots of little things.
Jan> +mi_command_py::mi_command_py (const char *name, int *suppress_notification,
Jan> + void *object)
Jan> + : mi_command (name, suppress_notification),
Jan> + pyobj (object)
Jan> +{}
I think it would be better to put this sort of thing somewhere in the
Python code, so that it can use gdbpy_ref<> rather than "void *".
Jan> + py_mi_invoke (this->pyobj, parse->argv, parse->argc);
This sort of indirection wouldn't be needed then either.
Jan> +class mi_command_py : public mi_command
Jan> +{
Jan> + public:
Jan> + mi_command_py (const char *name, int *suppress_notification,
Jan> + void *object);
Jan> + void invoke (struct mi_parse *parse) override;
Jan> +
Jan> + private:
Jan> + void *pyobj;
Jan> +
Jan> +};
Extra blank line before the "}".
Jan> diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
Jan> new file mode 100644
Jan> index 0000000000..ee612e2bc5
Jan> --- /dev/null
Jan> +++ b/gdb/python/py-micmd.c
Jan> @@ -0,0 +1,290 @@
Jan> +/* gdb MI commands implemented in Python */
Jan> +
Jan> +#include <string.h>
Files need a copyright header.
Jan> +#include "defs.h"
defs.h must come first. I wonder if string.h is needed at all.
Jan> +/* If the command invoked returns a list, this function parses it and create an
Jan> + appropriate MI out output.
Jan> +
Jan> + The returned values must be Python string, and can be contained within Python
Jan> + lists and dictionaries. It is possible to have a multiple levels of lists
Jan> + and/or dictionaries. */
Jan> +
Jan> +static void
Jan> +parse_mi_result (PyObject *result, const char *field_name)
Jan> +{
Jan> + struct ui_out *uiout = current_uiout;
Jan> +
Jan> + if(!field_name)
gdb+gnu style would be: "if (field_name == nullptr)"
Jan> + if (PyString_Check(result))
Need a space before open parens.
There are a few instances of this.
Jan> + {
Jan> + const char *string = gdbpy_obj_to_string (result).release ();
Jan> + uiout->field_string (field_name, string);
Jan> + xfree ( (void *)string);
Better to take advantage of the unique pointer, like:
gdb::unique_xmalloc_ptr<char> string = gdbpy_obj_to_string (result);
uiout->field_string (field_name, string.get ());
However, this code is ignoring Python errors. Probably this function
should use one of the standard conventions and return -1 on error.
Jan> + //struct cleanup *cleanup_item = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
There are a couple of these; remove them.
Jan> + item = PyList_GetItem (result, i);
This can fail, so it needs a check.
Jan> + else if ( PyDict_Check (result))
There's an extra space after paren.
Jan> + char *key_string = gdbpy_obj_to_string (key).release ();
Error checking.
Jan> + xfree ( (void *) key_string);
Again, no need to do this explicitly.
Jan> + PyObject *argobj, *result, **strings;
Extra space.
Jan> + if (! obj)
Jan> + error(_("Invalid invocation of Python micommand object."));
This could probably be improved somehow, for instance mentioning the MI
command name. I think that's sort of standard in MI commands, so this
should follow it in all the errors, I think.
Jan> + strings = (PyObject **) malloc (sizeof(PyObject *) * argc);
Not sure this is needed, see below. But if it is it should use XNEWVEC
or xmalloc.
Jan> + for (i = 0; i < argc; ++i)
Jan> + {
Jan> + strings[i] = PyUnicode_Decode (argv[i], strlen(argv[i]), host_charset (), NULL);
Jan> + if (PyList_SetItem (argobj, i, strings[i]) != 0)
PyList_SetItem steals a reference, so I think the ref-counting in this
function is incorrect. But, since it steals a reference, I think
"strings" isn't needed.
Jan> + {
Jan> + error (_("Failed to create the python arguments list."));
This will leak references. However if you use gdbpy_ref<>, you should
be ok.
Jan> + /* Skip preceding whitespaces. */
Jan> + /* Find first character of the final word. */
Not sure if the first comment documents something that should be there
and is not, or if it should just be removed.
Probably the latter, I think it's fine to require command names to be
valid. I'd probably skip stripping the trailing whitespace as well.
Jan> + cmd_name = micmdpy_parse_command_name (name);
Jan> + if (! cmd_name)
Jan> + return -1;
Like this could just give an error if the NAME isn't a valid MI command
name, and not bother with trying to fix it up, or copy it.
Jan> + PyErr_Format (except.reason == RETURN_QUIT
Jan> + ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
Jan> + "%s", except.message);
I think GDB_PY_SET_HANDLE_EXCEPTION is better for this.
Jan> +/* Initialize the MI command object. */
Jan> +
Jan> +int
Jan> +gdbpy_initialize_micommands(void)
Don't need (void) any more.
Jan> +++ b/gdb/python/py-micmd.h
Jan> @@ -0,0 +1,6 @@
Jan> +#ifndef PY_MICMDS_H
Jan> +#define PY_MICMDS_H
Jan> +
Jan> +void py_mi_invoke (void *py_obj, char **argv, int argc);
Jan> +
I'd just stick this in python-internal.h and not have a new header.
thanks,
Tom
@@ -382,6 +382,7 @@ SUBDIR_PYTHON_SRCS = \
python/py-instruction.c \
python/py-lazy-string.c \
python/py-linetable.c \
+ python/py-micmd.c \
python/py-newobjfileevent.c \
python/py-objfile.c \
python/py-param.c \
@@ -23,6 +23,7 @@
#include "mi-cmds.h"
#include "mi-main.h"
#include "mi-parse.h"
+#include "python/py-micmd.h"
#include <map>
#include <string>
#include <memory>
@@ -31,10 +32,9 @@
static std::map<std::string, mi_cmd_up> mi_cmd_table;
-/* Insert a new mi-command into the command table. Return true if
- insertion was successful. */
+/* See mi-cmds.h. */
-static bool
+bool
insert_mi_cmd_entry (mi_cmd_up command)
{
gdb_assert (command != NULL);
@@ -131,6 +131,27 @@ mi_command_cli::invoke (struct mi_parse *parse)
parse->args);
}
+mi_command_py::mi_command_py (const char *name, int *suppress_notification,
+ void *object)
+ : mi_command (name, suppress_notification),
+ pyobj (object)
+{}
+
+void
+mi_command_py::invoke (struct mi_parse *parse)
+{
+ std::unique_ptr<scoped_restore_tmpl <int>> restore
+ = do_suppress_notification ();
+
+ mi_parse_argv (parse->args, parse);
+
+ if (parse->argv == NULL)
+ error (_("Problem parsing arguments: %s %s"), parse->command, parse->args);
+
+ py_mi_invoke (this->pyobj, parse->argv, parse->argc);
+
+}
+
/* Initialize the available MI commands. */
static void
@@ -179,6 +179,20 @@ class mi_command_cli : public mi_command
int m_args_p;
};
+/* MI command implemented on top of a Python command. */
+
+class mi_command_py : public mi_command
+{
+ public:
+ mi_command_py (const char *name, int *suppress_notification,
+ void *object);
+ void invoke (struct mi_parse *parse) override;
+
+ private:
+ void *pyobj;
+
+};
+
typedef std::unique_ptr<mi_command> mi_cmd_up;
/* Lookup a command in the MI command table. */
@@ -190,4 +204,9 @@ extern int mi_debug_p;
extern void mi_execute_command (const char *cmd, int from_tty);
+/* Insert a new mi-command into the command table. Return true if
+ insertion was successful. */
+
+extern bool insert_mi_cmd_entry (mi_cmd_up command);
+
#endif /* MI_MI_CMDS_H */
new file mode 100644
@@ -0,0 +1,290 @@
+/* gdb MI commands implemented in Python */
+
+#include <string.h>
+
+#include "defs.h"
+#include "arch-utils.h"
+#include "value.h"
+#include "python-internal.h"
+#include "charset.h"
+#include "gdbcmd.h"
+#include "cli/cli-decode.h"
+#include "completer.h"
+#include "language.h"
+#include "mi/mi-cmds.h"
+
+struct micmdpy_object
+{
+ PyObject_HEAD
+};
+
+typedef struct micmdpy_object micmdpy_object;
+
+static PyObject *invoke_cst;
+
+extern PyTypeObject micmdpy_object_type
+ CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("micmdpy_object");
+
+/* If the command invoked returns a list, this function parses it and create an
+ appropriate MI out output.
+
+ The returned values must be Python string, and can be contained within Python
+ lists and dictionaries. It is possible to have a multiple levels of lists
+ and/or dictionaries. */
+
+static void
+parse_mi_result (PyObject *result, const char *field_name)
+{
+ struct ui_out *uiout = current_uiout;
+
+ if(!field_name)
+ field_name = "default";
+
+ if (PyString_Check(result))
+ {
+ const char *string = gdbpy_obj_to_string (result).release ();
+ uiout->field_string (field_name, string);
+ xfree ( (void *)string);
+ }
+ else if (PyList_Check (result))
+ {
+ PyObject *item;
+ Py_ssize_t i = 0;
+ ui_out_emit_list list_emitter (uiout, field_name);
+ for(i = 0; i < PyList_GET_SIZE (result); ++i)
+ {
+ //struct cleanup *cleanup_item = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+ ui_out_emit_tuple tuple_emitter (uiout, NULL);
+ item = PyList_GetItem (result, i);
+ parse_mi_result (item, NULL);
+ //do_cleanups (cleanup_item);
+ }
+ }
+ else if ( PyDict_Check (result))
+ {
+ PyObject *key, *value;
+ Py_ssize_t pos = 0;
+ while ( PyDict_Next (result, &pos, &key, &value) )
+ {
+ char *key_string = gdbpy_obj_to_string (key).release ();
+ parse_mi_result (value, key_string);
+ xfree ( (void *) key_string);
+ }
+ }
+}
+
+/* Called from mi_command_py's invoke to invoke the command. */
+
+void
+py_mi_invoke (void *py_obj, char **argv, int argc)
+{
+ micmdpy_object *obj = (micmdpy_object *) py_obj;
+ PyObject *argobj, *result, **strings;
+ int i;
+
+ gdbpy_enter enter_py (get_current_arch (), current_language);
+
+ if (! obj)
+ error(_("Invalid invocation of Python micommand object."));
+
+ if (! PyObject_HasAttr ((PyObject *) obj, invoke_cst))
+ {
+ error (_("Python command object missing 'invoke' method."));
+ }
+
+ strings = (PyObject **) malloc (sizeof(PyObject *) * argc);
+ argobj = PyList_New (argc);
+ if ( !argobj)
+ {
+ gdbpy_print_stack ();
+ error (_("Failed to create the python arguments list."));
+ }
+
+ for (i = 0; i < argc; ++i)
+ {
+ strings[i] = PyUnicode_Decode (argv[i], strlen(argv[i]), host_charset (), NULL);
+ if (PyList_SetItem (argobj, i, strings[i]) != 0)
+ {
+ error (_("Failed to create the python arguments list."));
+ }
+ }
+
+ result = PyObject_CallMethodObjArgs ((PyObject *) obj, invoke_cst, argobj,
+ NULL);
+
+ if (result)
+ {
+ parse_mi_result (result, NULL);
+ Py_DECREF (result);
+ }
+
+ Py_DECREF (argobj);
+ for (i = 0; i < argc; ++i)
+ {
+ Py_DECREF (strings[i]);
+ }
+ free (strings);
+}
+
+/* Parse the name of the MI command to register.
+
+ This function returns the xmalloc()d name of the new command. On error
+ sets the Python error and returns NULL. */
+
+static char *
+micmdpy_parse_command_name (const char *name)
+{
+ int len = strlen (name);
+ int i, lastchar;
+ char *result;
+
+ /* Skip trailing whitespaces. */
+ for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
+ ;
+ if (i < 0)
+ {
+ PyErr_SetString (PyExc_RuntimeError, _("No command name found."));
+ return NULL;
+ }
+ lastchar = i;
+
+ /* Skip preceding whitespaces. */
+ /* Find first character of the final word. */
+ for (; i > 0 && (isalnum (name[i - 1])
+ || name[i - 1] == '-'
+ || name[i - 1] == '_');
+ --i)
+ ;
+ /* Skip the first dash to have to command name only.
+ * i.e. -thread-info -> thread-info
+ */
+ if(name[i] == '-' && i < len - 2)
+ i++;
+
+ if( i == lastchar)
+ {
+ PyErr_SetString (PyExc_RuntimeError, _("No command name found."));
+ return NULL;
+ }
+
+ result = (char *) xmalloc (lastchar - i + 2);
+ memcpy(result, &name[i], lastchar - i + 1);
+ result[lastchar - i + 1] = '\0';
+
+ return result;
+}
+
+/* Object initializer; sets up gdb-side structures for MI command.
+
+ Use: __init__(NAME).
+
+ NAME is the name of the MI command to register. It should starts with a dash
+ as a traditional MI command does. */
+
+static int
+micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
+{
+ const char *name;
+ char *cmd_name;
+
+ if(! PyArg_ParseTuple (args, "s", &name))
+ return -1;
+
+ cmd_name = micmdpy_parse_command_name (name);
+ if (! cmd_name)
+ return -1;
+
+ Py_INCREF (self);
+
+ try
+ {
+ mi_command *micommand = new mi_command_py (cmd_name, NULL, (void *) self);
+
+ bool result = insert_mi_cmd_entry (mi_cmd_up (micommand));
+
+ if (! result)
+ {
+ PyErr_Format (PyExc_RuntimeError,
+ _("Unable to insert command. The name might already be in use."));
+ return -1;
+ }
+ }
+ catch (const gdb_exception &except)
+ {
+ xfree (cmd_name);
+ Py_DECREF (self);
+ PyErr_Format (except.reason == RETURN_QUIT
+ ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
+ "%s", except.message);
+ return -1;
+ }
+
+ xfree (cmd_name);
+ return 0;
+}
+
+/* Initialize the MI command object. */
+
+int
+gdbpy_initialize_micommands(void)
+{
+ micmdpy_object_type.tp_new = PyType_GenericNew;
+ if (PyType_Ready (&micmdpy_object_type) < 0)
+ return -1;
+
+ if (gdb_pymodule_addobject (gdb_module, "MICommand",
+ (PyObject *) &micmdpy_object_type) < 0)
+ return -1;
+
+ invoke_cst = PyString_FromString("invoke");
+ if (invoke_cst == NULL)
+ return -1;
+
+ return 0;
+}
+
+static PyMethodDef micmdpy_object_methods[] =
+ {
+ { 0 }
+ };
+
+PyTypeObject micmdpy_object_type =
+{
+ PyVarObject_HEAD_INIT (NULL, 0)
+ "gdb.MICommand", /*tp_name*/
+ sizeof (micmdpy_object), /*tp_basicsize*/
+ 0, /*tp_itemsize*/
+ 0, /*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*/
+ "GDB mi-command object", /* tp_doc */
+ 0, /* tp_traverse */
+ 0, /* tp_clear */
+ 0, /* tp_richcompare */
+ 0, /* tp_weaklistoffset */
+ 0, /* tp_iter */
+ 0, /* tp_iternext */
+ micmdpy_object_methods, /* tp_methods */
+ 0, /* tp_members */
+ 0, /* tp_getset */
+ 0, /* tp_base */
+ 0, /* tp_dict */
+ 0, /* tp_descr_get */
+ 0, /* tp_descr_set */
+ 0, /* tp_dictoffset */
+ micmdpy_init, /* tp_init */
+ 0, /* tp_alloc */
+};
new file mode 100644
@@ -0,0 +1,6 @@
+#ifndef PY_MICMDS_H
+#define PY_MICMDS_H
+
+void py_mi_invoke (void *py_obj, char **argv, int argc);
+
+#endif
@@ -539,6 +539,8 @@ int gdbpy_initialize_xmethods (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_unwind (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_micommands (void)
+ CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
/* A wrapper for PyErr_Fetch that handles reference counting for the
caller. */
@@ -1698,7 +1698,8 @@ do_start_initialization ()
|| gdbpy_initialize_event () < 0
|| gdbpy_initialize_arch () < 0
|| gdbpy_initialize_xmethods () < 0
- || gdbpy_initialize_unwind () < 0)
+ || gdbpy_initialize_unwind () < 0
+ || gdbpy_initialize_micommands () < 0)
return false;
#define GDB_PY_DEFINE_EVENT_TYPE(name, py_name, doc, base) \