Re: [PATCH 07/11 v5] Introduce get_thread_regcache_for_ptid

[Doug Evans <dje at google dot com>]

Gary Benson writes:
 > This introduces get_thread_regcache_for_ptid so that we can simplify
 > nat/linux-btrace.c.  A better long term solution would be unify the
 > regcache code, but this is sufficient for now.
 > 
 > gdb/
 > 2014-08-01  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* common/common-regcache.h: New file.
 > 	* Makefile.in (HFILES_NO_SRCDIR): Add common/common-regcache.h.
 > 	* regcache.c: Include the above.
 > 	(get_thread_regcache_for_ptid): New function.
 > 	* nat/linux-btrace.c: Include common-regcache.h.
 > 	(perf_event_read_bts): Use get_thread_regcache_for_ptid.
 > 
 > gdb/gdbserver/
 > 2014-08-01  Tom Tromey  <tromey@redhat.com>
 > 	    Gary Benson  <gbenson@redhat.com>
 > 
 > 	* regcache.c: Include common-regcache.h.
 > 	(get_thread_regcache_for_ptid): New function.
 > [...]
 > diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h
 > new file mode 100644
 > index 0000000..2ef7cb1
 > --- /dev/null
 > +++ b/gdb/common/common-regcache.h
 > @@ -0,0 +1,28 @@
 > +/* Cache and manage the values of registers
 > +
 > +   Copyright (C) 1986-2014 Free Software Foundation, Inc.
 > +
 > +   This file is part of GDB.
 > +
 > +   This program is free software; you can redistribute it and/or modify
 > +   it under the terms of the GNU General Public License as published by
 > +   the Free Software Foundation; either version 3 of the License, or
 > +   (at your option) any later version.
 > +
 > +   This program is distributed in the hope that it will be useful,
 > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
 > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 > +   GNU General Public License for more details.
 > +
 > +   You should have received a copy of the GNU General Public License
 > +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 > +
 > +#ifndef COMMON_REGCACHE_H
 > +#define COMMON_REGCACHE_H
 > +
 > +/* A hack until we have an independent regcache.  This must be
 > +   provided by the user.  */

This should still say what the function does,
what the inputs/outputs are, etc.

 > +
 > +extern struct regcache *get_thread_regcache_for_ptid (ptid_t ptid);
 > +
 > +#endif /* COMMON_REGCACHE_H */
 > diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c
 > index db9dad9..ec4f6c7 100644
 > --- a/gdb/gdbserver/regcache.c
 > +++ b/gdb/gdbserver/regcache.c
 > @@ -21,6 +21,8 @@
 >  #include "gdbthread.h"
 >  #include "tdesc.h"
 >  #include "rsp-low.h"
 > +#include "common-regcache.h"
 > +
 >  #ifndef IN_PROCESS_AGENT
 >  
 >  struct regcache *
 > @@ -61,6 +63,14 @@ get_thread_regcache (struct thread_info *thread, int fetch)
 >    return regcache;
 >  }
 >  
 > +/* See common/linux-btrace.h.  */

It's nat/linux-btrace.h, but I think you mean common/common-regcache.h.
[setting aside the #include "common/regcache.h" discussion]

 > +
 > +struct regcache *
 > +get_thread_regcache_for_ptid (ptid_t ptid)
 > +{
 > +  return get_thread_regcache (find_thread_ptid (ptid), 1);
 > +}
 > +
 >  void
 >  regcache_invalidate_thread (struct thread_info *thread)
 >  {
 > diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
 > index f6fdbda..b32d04c 100644
 > --- a/gdb/nat/linux-btrace.c
 > +++ b/gdb/nat/linux-btrace.c
 > @@ -30,6 +30,7 @@
 >  #include "gdbthread.h"
 >  #include "gdb_wait.h"
 >  #include "i386-cpuid.h"
 > +#include "common-regcache.h"
 >  
 >  #ifdef HAVE_SYS_SYSCALL_H
 >  #include <sys/syscall.h>
 > @@ -180,11 +181,7 @@ perf_event_read_bts (struct btrace_target_info* tinfo, const uint8_t *begin,
 >    gdb_assert (start <= end);
 >  
 >    /* The first block ends at the current pc.  */
 > -#ifdef GDBSERVER
 > -  regcache = get_thread_regcache (find_thread_ptid (tinfo->ptid), 1);
 > -#else
 > -  regcache = get_thread_regcache (tinfo->ptid);
 > -#endif
 > +  regcache = get_thread_regcache_for_ptid (tinfo->ptid);
 >    block.end = regcache_read_pc (regcache);
 >  
 >    /* The buffer may contain a partial record as its last entry (i.e. when the
 > diff --git a/gdb/regcache.c b/gdb/regcache.c
 > index f410dc2..2ffc121 100644
 > --- a/gdb/regcache.c
 > +++ b/gdb/regcache.c
 > @@ -28,6 +28,7 @@
 >  #include "exceptions.h"
 >  #include "remote.h"
 >  #include "valprint.h"
 > +#include "common-regcache.h"
 >  
 >  /*
 >   * DATA STRUCTURE
 > @@ -535,6 +536,13 @@ get_current_regcache (void)
 >    return get_thread_regcache (inferior_ptid);
 >  }
 >  
 > +/* See common/linux-btrace.h.  */

Similarly, common/common-regcache.h.

 > +
 > +struct regcache *
 > +get_thread_regcache_for_ptid (ptid_t ptid)
 > +{
 > +  return get_thread_regcache (ptid);
 > +}
 >  
 >  /* Observer for the target_changed event.  */
 >  
 > -- 
 > 1.7.1
 > 

Ok with those nits fixed.