[RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

remote-mips.c no longer builds because of this call to regcache_set_valid_p.
I couldn't find any trace of that routine in our ChangeLogs and I'm right
in the middle of a rather large change in my git directory, so I didn't
bother investigate more about this routine.

I think that the routine that we want, right now, is called
regcache_invalidate.

2009-03-16  Joel Brobecker  <brobecker@adacore.com>

        * remote-mips.c (mips_load): Replace call to regcache_set_valid_p, 
        which is undefined, by call to regcache_invalidate, which should
        do what the original author wanted to do.

Does this look plausible to anyone else? I'd just like a second pair
of eyes before I commit.

Thank you!
-- 
Joel

[-- Attachment #2: remote-mips.c.diff --]
[-- Type: text/x-diff, Size: 825 bytes --]

Index: remote-mips.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-mips.c,v
retrieving revision 1.97
diff -u -p -r1.97 remote-mips.c
--- remote-mips.c	16 Mar 2009 19:38:32 -0000	1.97
+++ remote-mips.c	16 Mar 2009 19:50:15 -0000
@@ -3276,9 +3276,9 @@ mips_load (char *file, int from_tty)
          to a different value than GDB thinks it has. The following ensures
          that the write_pc() WILL update the PC value: */
       struct regcache *regcache = get_current_regcache ();
-      regcache_set_valid_p (regcache,
-			    gdbarch_pc_regnum (get_regcache_arch (regcache)),
-					       0);
+
+      regcache_invalidate (regcache,
+			   gdbarch_pc_regnum (get_regcache_arch (regcache)));
     }
   if (exec_bfd)
     write_pc (bfd_get_start_address (exec_bfd));

Re: [RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Daniel Jacobowitz]

On Mon, Mar 16, 2009 at 03:54:30PM -0400, Joel Brobecker wrote:
> remote-mips.c no longer builds because of this call to regcache_set_valid_p.
> I couldn't find any trace of that routine in our ChangeLogs and I'm right
> in the middle of a rather large change in my git directory, so I didn't
> bother investigate more about this routine.
> 
> I think that the routine that we want, right now, is called
> regcache_invalidate.
> 
> 2009-03-16  Joel Brobecker  <brobecker@adacore.com>
> 
>         * remote-mips.c (mips_load): Replace call to regcache_set_valid_p, 
>         which is undefined, by call to regcache_invalidate, which should
>         do what the original author wanted to do.
> 
> Does this look plausible to anyone else? I'd just like a second pair
> of eyes before I commit.

Does to me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Pedro Alves]

On Monday 16 March 2009 19:54:30, Joel Brobecker wrote:
> remote-mips.c no longer builds because of this call to regcache_set_valid_p.
> I couldn't find any trace of that routine in our ChangeLogs and I'm right
> in the middle of a rather large change in my git directory, so I didn't
> bother investigate more about this routine.
> 
> I think that the routine that we want, right now, is called
> regcache_invalidate.
> 

I believe this was Maciej's change.

> 2009-03-16  Joel Brobecker  <brobecker@adacore.com>
> 
>         * remote-mips.c (mips_load): Replace call to regcache_set_valid_p, 
>         which is undefined, by call to regcache_invalidate, which should
>         do what the original author wanted to do.
> 
> Does this look plausible to anyone else? I'd just like a second pair
> of eyes before I commit.
> 

We have this in an internal MIPS tree:

void
regcache_invalidate (struct regcache *regcache, int regnum)
{
  regcache_set_valid_p (regcache, regnum, 0);
}

So, it looks correct.  This file isn't used by any FSF
configuration, BTW.  I don't think I converted it to always a
thread, so it surely isn't working as is.

-- 
Pedro Alves

Re: [RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Joel Brobecker]

> 2009-03-16  Joel Brobecker  <brobecker@adacore.com>
> 
>         * remote-mips.c (mips_load): Replace call to regcache_set_valid_p, 
>         which is undefined, by call to regcache_invalidate, which should
>         do what the original author wanted to do.

Thank you for the extra pair of eyes, guys. Patch now checked in.

I was afraid that this might not be sufficient to make the remote-mips
fully functional (Pedro mentioned converting this target to
always-a-thread), but at least we're trying to move in the right
direction...

-- 
Joel

Re: [RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Pedro Alves]

On Monday 16 March 2009 21:15:07, Joel Brobecker wrote:

> I was afraid that this might not be sufficient to make the remote-mips
> fully functional (Pedro mentioned converting this target to
> always-a-thread), but at least we're trying to move in the right
> direction...
> 

Ah, I knew I had been there before:

 http://sourceware.org/ml/gdb-patches/2008-08/msg00475.html

-- 
Pedro Alves

Re: [RFA/commit] undefined regcache_set_valid_p in remote-mips.c

[Joel Brobecker]

> Ah, I knew I had been there before:
> 
>  http://sourceware.org/ml/gdb-patches/2008-08/msg00475.html

FWIW, I wonder what's best in this case:
  (1) Leave the code as is, fearing that we might break something,
      but with a strong suspicion that it is broken by an earlier change;
  (2) Try fixing it blindly using visual code inspection for testing.

Short of being able to test the change ourselves, if you think as I do
that the target is very likely broken already, our best is all we have
to try to fix it.

The alternative is to make a call for help from people who care about
this target, and see if anyone is willing to help, at least with
the testing and integration.  Otherwise, we could consider deprecating
this target. It's always a shame to lose a target, but what can we do
if no one if we don't have the means to keep it alive?

-- 
Joel