This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[python] RFC: don't allow deletion of convenience function


Right now if the user creates a convenience function and then deletes
it (by overwriting the variable: set $func = 0), gdb will leak memory.
Also, gdb will leave around the "function whatever" dummy command
(which is used for help).

This patch is one way to solve these problems: it disallows deleting
the canonical name of a convenience function.

This seems ok-ish to me because there is no way to delete a gdb
command, either, and that doesn't seem to be a big problem.

Still, I'd appreciate any comments before I commit this.

Tom

2009-03-05  Tom Tromey  <tromey@redhat.com>

	* python/python-function.c (fnpy_destroy): Remove.
	* value.c (create_internalvar): Initialize new field.
	(set_internalvar): Don't allow assignment to canonical variable.
	(struct internal_function) <destroyer>: Remove.
	(value_create_internal_function): Remove 'destroyer' argument.
	(add_internal_function): Likewise.  Set variable's 'canonical'
	field.
	* value.h (struct internalvar) <canonical>: New field.
	(add_internal_function): Update.

diff --git a/gdb/python/python-function.c b/gdb/python/python-function.c
index 5e346d6..608ac28 100644
--- a/gdb/python/python-function.c
+++ b/gdb/python/python-function.c
@@ -1,6 +1,6 @@
 /* Convenience functions implemented in Python.
 
-   Copyright (C) 2008 Free Software Foundation, Inc.
+   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -97,19 +97,6 @@ fnpy_call (void *cookie, int argc, struct value **argv)
   return value;
 }
 
-/* Called when destroying a struct internal_function.  */
-
-static void
-fnpy_destroy (void *cookie)
-{
-  PyGILState_STATE state;
-
-  state = PyGILState_Ensure ();
-  Py_DECREF ((PyObject *) cookie);
-  PyGILState_Release (state);
-
-}
-
 /* Initializer for a Function object.  It takes one argument, the name
    of the function.  */
 
@@ -131,7 +118,7 @@ fnpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   if (! docstring)
     docstring = _("This function is not documented.");
 
-  add_internal_function (name, docstring, fnpy_call, self, fnpy_destroy);
+  add_internal_function (name, docstring, fnpy_call, self);
   return 0;
 }
 
diff --git a/gdb/value.c b/gdb/value.c
index ac55685..fd05f07 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -58,10 +58,6 @@ struct internal_function
 
   /* User data for the handler.  */
   void *cookie;
-
-  /* Function called to destroy the cookie when the function object is
-     destroyed.  */
-  void (*destroyer) (void *cookie);
 };
 
 static struct cmd_list_element *functionlist;
@@ -879,6 +875,7 @@ create_internalvar (const char *name)
   var->name = concat (name, (char *)NULL);
   var->value = allocate_value (builtin_type_void);
   var->endian = gdbarch_byte_order (current_gdbarch);
+  var->canonical = 0;
   release_value (var->value);
   var->next = internalvars;
   internalvars = var;
@@ -969,6 +966,9 @@ set_internalvar (struct internalvar *var, struct value *val)
 {
   struct value *newval;
 
+  if (var->canonical)
+    error (_("Cannot overwrite convenience function %s"), var->name);
+
   newval = value_copy (val);
   newval->modifiable = 1;
 
@@ -1000,22 +1000,15 @@ internalvar_name (struct internalvar *var)
 static struct value *
 value_create_internal_function (const char *name,
 				internal_function_fn handler,
-				void *cookie,
-				void (*destroyer) (void *))
+				void *cookie)
 {
   struct value *result = allocate_value (internal_fn_type);
   gdb_byte *addr = value_contents_writeable (result);
   struct internal_function **fnp = (struct internal_function **) addr;
-  /* The internal_function object is leaked here -- to make it truly
-     deletable, we would have to reference count it and add special
-     code to value_free and value_copy.  The setup here is a bit odd
-     in general.  It would be better to have a special case in
-     help_command.  */
   struct internal_function *ifn = XNEW (struct internal_function);
   ifn->name = xstrdup (name);
   ifn->handler = handler;
   ifn->cookie = cookie;
-  ifn->destroyer = destroyer;
   *fnp = ifn;
   return result;
 }
@@ -1057,19 +1050,17 @@ function_destroyer (struct cmd_list_element *self, void *ignore)
 /* Add a new internal function.  NAME is the name of the function; DOC
    is a documentation string describing the function.  HANDLER is
    called when the function is invoked.  COOKIE is an arbitrary
-   pointer which is passed to HANDLER and is intended for "user data".
-   DESTROYER is invoked when the function is destroyed.  */
+   pointer which is passed to HANDLER and is intended for "user
+   data".  */
 void
 add_internal_function (const char *name, const char *doc,
-		       internal_function_fn handler,
-		       void *cookie, void (*destroyer) (void *))
+		       internal_function_fn handler, void *cookie)
 {
   struct cmd_list_element *cmd;
   struct internalvar *var = lookup_internalvar (name);
-  struct value *fnval = value_create_internal_function (name, handler, cookie,
-							destroyer);
-  release_value (fnval);
+  struct value *fnval = value_create_internal_function (name, handler, cookie);
   set_internalvar (var, fnval);
+  var->canonical = 1;
 
   cmd = add_cmd (xstrdup (name), no_class, function_command, (char *) doc,
 		 &functionlist);
diff --git a/gdb/value.h b/gdb/value.h
index c8a4489..fd0d2c9 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -259,6 +259,9 @@ struct internalvar
   char *name;
   struct value *value;
   int endian;
+  /* True if this internalvar is the canonical name for a convenience
+     function.  */
+  int canonical;
 };
 
 
@@ -612,7 +615,7 @@ typedef struct value *(*internal_function_fn) (void *cookie,
 
 void add_internal_function (const char *name, const char *doc,
 			    internal_function_fn handler,
-			    void *cookie, void (*destroyer) (void *));
+			    void *cookie);
 
 struct value *call_internal_function (struct value *function,
 				      int argc, struct value **argv);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]