Re: RFA: remote_address_size changes

Re: RFA: remote_address_size changes

[Kevin Buettner]

On Nov 4,  8:57am, Andrew Cagney wrote:

> Have a look at how remote.c registers and then updates the variable
> tty_input.  When ever the user selects a new target, that variable is
> switched (brutal yes).
> 
> For remote_address_size you would probably want to do something similar.

I agree.  I've appended a new patch for you to look at below.

[...]
> The above mechanism binds ``remote_address_size'' to the architecture. 
> If the architecture changes - new binary of a different type then the
> old selection is lost.  Is this a good thing or a  dangerous thing?
> 
> Consider the sequence:
> 
> 	set remote address-size 16
> 	file xyz
> 
> which can have different semantics to:
> 
> 	file xyz
> 	set remote address-size 16
> 
> Making their behavour identical isn't trivial (fortunatly it isn't
> difficult either).  The problem is figuring out exactly what reasonable
> behavour is - I have no opinion.

I don't (necessarily) see a problem with overriding the users setting
in the following scenario.

 	set remote address-size 16
 	file xyz

Consider the following (more ambiguous) scenario:

 	file xyz
	...
 	set remote address-size 16
	...
 	file zyx

I think there will be situations where it definitely makes sense for
"file zyx" to cause a different remote_address_size to be set.  There
are also situations where it doesn't make sense; I think it will be
difficult to intelligently disabmiguate them.  In situations like
this, maybe it would make sense to warn the user when setting a new
target causes the user's setting to be overridden.  (Unfortunately,
this means that we have to keep track of whether or not the user set a
variable or not.)

Now on to the matter of naming...  I like the command names that you
used in your example sequences, e.g,

 	set remote address-size 16

But, at present, this notation doesn't work.  Instead you have to do

	set remoteaddresssize 16

I think we should add an alias so the notation in your examples will
work as well.  Also, I think your memory-{read,write}-packet-size
names are fine.  (It's a good thing we have name completion though, or
I might not like them so well.)

Here's the patch...

	* remote.c (build_remote_gdbarch_data): Set remote_address_size...
	(_initialize_remote) ...but don't set it here.  Also, tie
	remote_address_size to the target architecture via call to
	register_gdbarch_swap().

Index: remote.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/remote.c,v
retrieving revision 1.255
diff -u -r1.255 remote.c
--- remote.c	1999/11/04 11:04:51	1.255
+++ remote.c	1999/11/04 20:31:39
@@ -5221,6 +5221,7 @@
 build_remote_gdbarch_data ()
 {
   tty_input = xmalloc (PBUFSIZ);
+  remote_address_size = TARGET_PTR_BIT;
 }
 
 void
@@ -5232,6 +5233,8 @@
   /* architecture specific data */
   build_remote_gdbarch_data ();
   register_gdbarch_swap (&tty_input, sizeof (&tty_input), NULL);
+  register_gdbarch_swap (&remote_address_size, 
+                         sizeof (&remote_address_size), NULL);
   register_gdbarch_swap (NULL, 0, build_remote_gdbarch_data);
 
   /* runtime constants - we retain the value of remote_write_size
@@ -5305,7 +5308,6 @@
 		  &setlist),
      &showlist);
 
-  remote_address_size = TARGET_PTR_BIT;
   add_show_from_set
     (add_set_cmd ("remoteaddresssize", class_obscure,
 		  var_integer, (char *) &remote_address_size,

Re: RFA: remote_address_size changes

[Andrew Cagney]

Kevin Buettner wrote:

> I don't (necessarily) see a problem with overriding the users setting
> in the following scenario.
> 
>         set remote address-size 16
>         file xyz
> 
> Consider the following (more ambiguous) scenario:
> 
>         file xyz
>         ...
>         set remote address-size 16
>         ...
>         file zyx
> 
> I think there will be situations where it definitely makes sense for
> "file zyx" to cause a different remote_address_size to be set.  There
> are also situations where it doesn't make sense; I think it will be
> difficult to intelligently disabmiguate them.  In situations like
> this, maybe it would make sense to warn the user when setting a new
> target causes the user's setting to be overridden.  (Unfortunately,
> this means that we have to keep track of whether or not the user set a
> variable or not.)

Yes.  Notifying the user may be trick - really a policy issue for the
GDB architect.
The file command, already explicitly notifies the user that breakpoints
changed.

> Now on to the matter of naming...  I like the command names that you
> used in your example sequences, e.g,
> 
>         set remote address-size 16
> 
> But, at present, this notation doesn't work.  Instead you have to do
> 
>         set remoteaddresssize 16

That is the old way (just don't mention ``set *debug'' :-). The ``set
remote'' prefix was a very recent addition.

> I think we should add an alias so the notation in your examples will
> work as well.  Also, I think your memory-{read,write}-packet-size
> names are fine.  (It's a good thing we have name completion though, or
> I might not like them so well.)

Yes, fine with me.  (The names are long but I can't think of any shorter
but unambigious variants).

> Here's the patch...
> 
>         * remote.c (build_remote_gdbarch_data): Set remote_address_size...
>         (_initialize_remote) ...but don't set it here.  Also, tie
>         remote_address_size to the target architecture via call to
>         register_gdbarch_swap().

Again, fine - check it in.

	thanks,
		Andrew



> Index: remote.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/gdb/remote.c,v
> retrieving revision 1.255
> diff -u -r1.255 remote.c
> --- remote.c    1999/11/04 11:04:51     1.255
> +++ remote.c    1999/11/04 20:31:39
> @@ -5221,6 +5221,7 @@
>  build_remote_gdbarch_data ()
>  {
>    tty_input = xmalloc (PBUFSIZ);
> +  remote_address_size = TARGET_PTR_BIT;
>  }
> 
>  void
> @@ -5232,6 +5233,8 @@
>    /* architecture specific data */
>    build_remote_gdbarch_data ();
>    register_gdbarch_swap (&tty_input, sizeof (&tty_input), NULL);
> +  register_gdbarch_swap (&remote_address_size,
> +                         sizeof (&remote_address_size), NULL);
>    register_gdbarch_swap (NULL, 0, build_remote_gdbarch_data);
> 
>    /* runtime constants - we retain the value of remote_write_size
> @@ -5305,7 +5308,6 @@
>                   &setlist),
>       &showlist);
> 
> -  remote_address_size = TARGET_PTR_BIT;
>    add_show_from_set
>      (add_set_cmd ("remoteaddresssize", class_obscure,
>                   var_integer, (char *) &remote_address_size,
From muller@cerbere.u-strasbg.fr Thu Nov 04 14:48:00 1999
From: muller@cerbere.u-strasbg.fr
To: Stan Shebs <shebs@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: Watching complex expressions patch
Date: Thu, 04 Nov 1999 14:48:00 -0000
Message-id: <3.0.6.32.19991104235154.008b2e00@ics.u-strasbg.fr>
References: <199911041058.LAA04309@cerbere.u-strasbg.fr> <199911041950.LAA23185@andros.cygnus.com>
X-SW-Source: 1999-q4/msg00177.html
Content-length: 1041

At 11:50 04/11/99 -0800, Stan Shebs wrote:
>
>   Date: Thu, 04 Nov 1999 11:46:19 +0100
>   From: Pierre Muller <muller@cerbere.u-strasbg.fr>
>
>     So my mecanism remembers the last memory that has been set a watch 
>   and after reject all watches of bigger memory regions including that
memory !
>
>I'm not clear on what you're getting at here, but it sounds wrong.
>GDB should allow the user to set any combination of watchpoints on the
>same or related pieces of data.  The reason is the same as for
>breakpoints; it's possible to attach conditions to watchpoints, and
>you may have reason to attach one condition to "watch t.c", but a
>different condition to a "watch t".  GDB should evaluate both
>conditions whenever t.c changes, but only one condition each time t.a
>or t.b changes.

  No, you misunderstood me :
I was only talking about a val_clain for one specific watchpoint
Settings different watchpoint that overlap is not affected by my changes !!

 The last memory region is reset to zero 
for each watchpoint considered !