Re: RFC: selected frame in read_var_value

[Daniel Jacobowitz <drow at mvista dot com>]

On Fri, Sep 05, 2003 at 01:09:46PM -0400, Andrew Cagney wrote:
> >On Thu, Aug 07, 2003 at 01:45:28PM -0400, Andrew Cagney wrote:
> >
> >>>Thoughts on this?  I observed one case where the selected frame hadn't 
> >>been
> >>>initialized yet: when checking watchpoint values.  Oops!  The condition
> >>>matches the case in which get_current_frame () can be reasonably 
> >>expected >to
> >>>succeed.  There are a couple other places that need this same check, 
> >>mostly
> >>>functions which will work on globals as well as locals.
> >
> >>
> >>I think the caller, and not this code, should be the one deciding 
> >>if/when a get_selected_frame should be called.  Otherwize the dependance 
> >>on global state remains.
> >
> >
> >The callers don't know any more than we do.  Take a look at some of
> >them - several callers don't take frame arguments, and work both when
> >the program is running and when it isn't.  If you want to cut down on
> >the global state dependence, I guess you'd have to push this up through
> >every caller, so that it could eventually be pushed even higher?
> 
> Yep, welcome to my world :-)
> 
> >Me, I was just fixing the regression caused by lazily selecting a
> >frame.  I wasn't planning on redesigning interfaces all through the
> >block and variable layers; and fixing this really does go quite far up.
> >For instance it works back to block_innermost_frame, which can return
> >NULL.
> 
> What about something like this as a step sideways:
> 
> >if (frame == NULL)
> >  frame = deprecated_safe_get_selected_frame ();
> 
> where deprecated_safe_get_selected_frame() would contain those sanity 
> checks?
> 
> Andrew

Is this about what you wanted, and are my comments on the mark?

If so, how do you feel about a mass replacement of the one deprecated
construct (deprecated_selected_frame) with the new deprecated construct
(deprecated_safe_get_selected_frame) in the places in GDB which use
this "if frame arg is NULL, get selected frame" idiom?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-09-06  Daniel Jacobowitz  <drow@mvista.com>

	* frame.c (deprecated_safe_get_selected_frame): New function.
	* frame.h (deprecated_safe_get_selected_frame): Add prototype.
	* findvar.c (read_var_value): Call it.

Index: findvar.c
===================================================================
RCS file: /cvs/src/src/gdb/findvar.c,v
retrieving revision 1.62
diff -u -p -r1.62 findvar.c
--- findvar.c	22 Jul 2003 15:41:59 -0000	1.62
+++ findvar.c	7 Sep 2003 03:46:36 -0000
@@ -404,8 +404,11 @@ read_var_value (register struct symbol *
 
   len = TYPE_LENGTH (type);
 
+
+  /* FIXME drow/2003-09-06: this call to the selected frame should be
+     pushed upwards to the callers.  */
   if (frame == NULL)
-    frame = deprecated_selected_frame;
+    frame = deprecated_safe_get_selected_frame ();
 
   switch (SYMBOL_CLASS (var))
     {
Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.137
diff -u -p -r1.137 frame.c
--- frame.c	12 Aug 2003 17:45:13 -0000	1.137
+++ frame.c	7 Sep 2003 03:46:36 -0000
@@ -917,6 +917,18 @@ get_selected_frame (void)
   return deprecated_selected_frame;
 }
 
+/* This is a variant of get_selected_frame which can be called when the
+   inferior is not running; in that case it will return NULL instead of
+   calling error ().  */
+
+struct frame_info *
+deprecated_safe_get_selected_frame (void)
+{
+  if (!target_has_registers || !target_has_stack || !target_has_memory)
+    return NULL;
+  return get_selected_frame ();
+}
+
 /* Select frame FI (or NULL - to invalidate the current frame).  */
 
 void
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.106
diff -u -p -r1.106 frame.h
--- frame.h	11 Jul 2003 15:31:43 -0000	1.106
+++ frame.h	7 Sep 2003 03:46:36 -0000
@@ -639,6 +639,19 @@ extern void return_command (char *, int)
 
 extern struct frame_info *deprecated_selected_frame;
 
+/* NOTE: drow/2003-09-06:
+
+   This function is "a step sideways" for uses of deprecated_selected_frame.
+   They should be fixed as above, but meanwhile, we needed a solution for
+   cases where functions are called with a NULL frame meaning either "the
+   program is not running" or "use the selected frame".  Lazy building of
+   deprecated_selected_frame confuses the situation, because now
+   deprecated_selected_frame can be NULL even when the inferior is running.
+
+   This function calls get_selected_frame if the inferior is running, or
+   returns NULL otherwise.  */
+
+extern struct frame_info *deprecated_safe_get_selected_frame (void);
 
 /* Create a frame using the specified BASE and PC.  */