* [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers
@ 2011-11-22 16:06 Maciej W. Rozycki
2011-12-06 19:14 ` Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Maciej W. Rozycki @ 2011-11-22 16:06 UTC (permalink / raw)
To: gdb-patches
Hi,
While working on a MIPS/Linux DSP support change I am about to submit I
have tripped over usr_fetch_inferior_registers and
usr_store_inferior_registers being structured in a different way even
though the make complementing actions and there's no obvious reason for
them to be designed like this. Specifically usr_fetch_inferior_registers
uses a helper to retrieve individual registers but
usr_store_inferior_registers recurses into itself instead.
The change below makes the two functions consistent,
usr_store_inferior_registers now uses a store_register helper exactly like
usr_fetch_inferior_registers uses fetch_register.
No regressions on i686-linux-gnu or mips-linux-gnu (I had to force
srv_linux_regsets to "no" and deal with some bitrot of that configuration
to get indicative results). OK to apply?
2011-11-22 Maciej W. Rozycki <macro@codesourcery.com>
gdb/gdbserver/
* linux-low.c (usr_store_inferior_registers): Factor out code
to handle individual registers into...
(store_register): ... this new function.
Maciej
gdb-gdbserver-linux-store-inferior.diff
Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c 2011-11-17 19:36:30.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c 2011-11-17 19:47:38.345612518 +0000
@@ -3768,6 +3768,60 @@ fetch_register (struct regcache *regcach
supply_register (regcache, regno, buf);
}
+/* Store one register. */
+static void
+store_register (struct regcache *regcache, int regno)
+{
+ CORE_ADDR regaddr;
+ int i, size;
+ char *buf;
+ int pid;
+
+ if (regno >= the_low_target.num_regs)
+ return;
+
+ if ((*the_low_target.cannot_store_register) (regno) == 1)
+ return;
+
+ regaddr = register_addr (regno);
+ if (regaddr == -1)
+ return;
+ errno = 0;
+ size = (register_size (regno) + sizeof (PTRACE_XFER_TYPE) - 1)
+ & - sizeof (PTRACE_XFER_TYPE);
+ buf = alloca (size);
+ memset (buf, 0, size);
+
+ if (the_low_target.collect_ptrace_register)
+ the_low_target.collect_ptrace_register (regcache, regno, buf);
+ else
+ collect_register (regcache, regno, buf);
+
+ pid = lwpid_of (get_thread_lwp (current_inferior));
+ for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
+ {
+ errno = 0;
+ ptrace (PTRACE_POKEUSER, pid,
+ /* Coerce to a uintptr_t first to avoid potential gcc warning
+ about coercing an 8 byte integer to a 4 byte pointer. */
+ (PTRACE_ARG3_TYPE) (uintptr_t) regaddr,
+ (PTRACE_ARG4_TYPE) *(PTRACE_XFER_TYPE *) (buf + i));
+ if (errno != 0)
+ {
+ /* At this point, ESRCH should mean the process is
+ already gone, in which case we simply ignore attempts
+ to change its registers. See also the related
+ comment in linux_resume_one_lwp. */
+ if (errno == ESRCH)
+ return;
+
+ if ((*the_low_target.cannot_store_register) (regno) == 0)
+ error ("writing register %d: %s", regno, strerror (errno));
+ }
+ regaddr += sizeof (PTRACE_XFER_TYPE);
+ }
+}
+
/* Fetch all registers, or just one, from the child process. */
static void
usr_fetch_inferior_registers (struct regcache *regcache, int regno)
@@ -3785,60 +3839,11 @@ usr_fetch_inferior_registers (struct reg
static void
usr_store_inferior_registers (struct regcache *regcache, int regno)
{
- CORE_ADDR regaddr;
- int i, size;
- char *buf;
- int pid;
-
- if (regno >= 0)
- {
- if (regno >= the_low_target.num_regs)
- return;
-
- if ((*the_low_target.cannot_store_register) (regno) == 1)
- return;
-
- regaddr = register_addr (regno);
- if (regaddr == -1)
- return;
- errno = 0;
- size = (register_size (regno) + sizeof (PTRACE_XFER_TYPE) - 1)
- & - sizeof (PTRACE_XFER_TYPE);
- buf = alloca (size);
- memset (buf, 0, size);
-
- if (the_low_target.collect_ptrace_register)
- the_low_target.collect_ptrace_register (regcache, regno, buf);
- else
- collect_register (regcache, regno, buf);
-
- pid = lwpid_of (get_thread_lwp (current_inferior));
- for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE))
- {
- errno = 0;
- ptrace (PTRACE_POKEUSER, pid,
- /* Coerce to a uintptr_t first to avoid potential gcc warning
- about coercing an 8 byte integer to a 4 byte pointer. */
- (PTRACE_ARG3_TYPE) (uintptr_t) regaddr,
- (PTRACE_ARG4_TYPE) *(PTRACE_XFER_TYPE *) (buf + i));
- if (errno != 0)
- {
- /* At this point, ESRCH should mean the process is
- already gone, in which case we simply ignore attempts
- to change its registers. See also the related
- comment in linux_resume_one_lwp. */
- if (errno == ESRCH)
- return;
-
- if ((*the_low_target.cannot_store_register) (regno) == 0)
- error ("writing register %d: %s", regno, strerror (errno));
- }
- regaddr += sizeof (PTRACE_XFER_TYPE);
- }
- }
- else
+ if (regno == -1)
for (regno = 0; regno < the_low_target.num_regs; regno++)
- usr_store_inferior_registers (regcache, regno);
+ store_register (regcache, regno);
+ else
+ store_register (regcache, regno);
}
#endif /* HAVE_LINUX_USRREGS */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers
2011-11-22 16:06 [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers Maciej W. Rozycki
@ 2011-12-06 19:14 ` Pedro Alves
2011-12-06 23:10 ` Maciej W. Rozycki
0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2011-12-06 19:14 UTC (permalink / raw)
To: gdb-patches; +Cc: Maciej W. Rozycki
On Tuesday 22 November 2011 16:06:09, Maciej W. Rozycki wrote:
> No regressions on i686-linux-gnu or mips-linux-gnu (I had to force
> srv_linux_regsets to "no" and deal with some bitrot of that configuration
> to get indicative results). OK to apply?
Okay. Thanks.
> 2011-11-22 Maciej W. Rozycki <macro@codesourcery.com>
>
> gdb/gdbserver/
> * linux-low.c (usr_store_inferior_registers): Factor out code
> to handle individual registers into...
> (store_register): ... this new function.
--
Pedro Alves
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers
2011-12-06 19:14 ` Pedro Alves
@ 2011-12-06 23:10 ` Maciej W. Rozycki
0 siblings, 0 replies; 3+ messages in thread
From: Maciej W. Rozycki @ 2011-12-06 23:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Tue, 6 Dec 2011, Pedro Alves wrote:
> > No regressions on i686-linux-gnu or mips-linux-gnu (I had to force
> > srv_linux_regsets to "no" and deal with some bitrot of that configuration
> > to get indicative results). OK to apply?
>
> Okay. Thanks.
>
> > 2011-11-22 Maciej W. Rozycki <macro@codesourcery.com>
> >
> > gdb/gdbserver/
> > * linux-low.c (usr_store_inferior_registers): Factor out code
> > to handle individual registers into...
> > (store_register): ... this new function.
Applied now, thanks for the review.
Maciej
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-12-06 23:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-22 16:06 [PATCH] Linux/gdbserver: Factor out usr_store_inferior_registers Maciej W. Rozycki
2011-12-06 19:14 ` Pedro Alves
2011-12-06 23:10 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox