From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24942 invoked by alias); 23 Feb 2012 22:06:34 -0000 Received: (qmail 24929 invoked by uid 22791); 23 Feb 2012 22:06:32 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Feb 2012 22:06:16 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1NM6C2i006922 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 23 Feb 2012 17:06:12 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q1NM6B6h009363; Thu, 23 Feb 2012 17:06:11 -0500 Message-ID: <4F46B852.2060502@redhat.com> Date: Thu, 23 Feb 2012 22:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120131 Thunderbird/10.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 7/9] move in_process_agent_loaded to agent_loaded_p. References: <1329447300-18841-1-git-send-email-yao@codesourcery.com> <1329447300-18841-8-git-send-email-yao@codesourcery.com> In-Reply-To: <1329447300-18841-8-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-02/txt/msg00522.txt.bz2 On 02/17/2012 02:54 AM, Yao Qi wrote: > This patch moves in_process_agent_loaded from gdbserver to gdb/common, > and renmame it to agent_loaded_p, then gdb can use it as well. This > is a refacotr patch, no function is changed. > > gdb: > > 2012-02-14 Yao Qi > > * common/agent.c (agent_loaded_p): New. > (agent_look_up_symbols): New global. > * common/agent.h: Declare agent_loaded_p. > > gdb/gdbserver: > > 2012-02-14 Yao Qi > > * Makefile.in (linux-low.o): Keep dependence on agent.h. > (linux-x86-low.o): Likewise. > * server.h: Remove in_process_agent_loaded. > * tracepoint.c (in_process_agent_loaded): Removed. Moved it > common/agent.c. > Update callers. > --- > gdb/common/agent.c | 11 +++++++++++ > gdb/common/agent.h | 2 ++ > gdb/gdbserver/Makefile.in | 4 ++-- > gdb/gdbserver/linux-low.c | 7 ++++--- > gdb/gdbserver/linux-x86-low.c | 3 ++- > gdb/gdbserver/server.h | 2 -- > gdb/gdbserver/tracepoint.c | 37 +++++++++++++------------------------ > 7 files changed, 34 insertions(+), 32 deletions(-) > > diff --git a/gdb/common/agent.c b/gdb/common/agent.c > index 074efea..d2f59db 100644 > --- a/gdb/common/agent.c > +++ b/gdb/common/agent.c > @@ -71,6 +71,14 @@ static struct > > static struct ipa_sym_addresses ipa_sym_addrs; > > +static int all_agent_symbols_looked_up = 0; > + > +int > +agent_loaded_p (void) > +{ > + return all_agent_symbols_looked_up; > +} > + > /* Look up all symbols needed by agent. Return 0 if all the symbols are > found, return non-zero otherwise. */ > > @@ -79,6 +87,8 @@ agent_look_up_symbols (void) > { > int i; > > + all_agent_symbols_looked_up = 0; > + > for (i = 0; i < sizeof (symbol_list) / sizeof (symbol_list[0]); i++) > { > CORE_ADDR *addrp = > @@ -100,6 +110,7 @@ agent_look_up_symbols (void) > } > } > > + all_agent_symbols_looked_up = 1; > return 0; > } > > diff --git a/gdb/common/agent.h b/gdb/common/agent.h > index a1ac9b2..b89d111 100644 > --- a/gdb/common/agent.h > +++ b/gdb/common/agent.h > @@ -33,6 +33,8 @@ int agent_look_up_symbols (void); > thread. */ > #define IPA_CMD_BUF_SIZE 1024 > > +int agent_loaded_p (void); > + > extern int debug_agent; > > extern int use_agent; > diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in > index e840db2..b7f1454 100644 > --- a/gdb/gdbserver/Makefile.in > +++ b/gdb/gdbserver/Makefile.in > @@ -451,7 +451,7 @@ i386-low.o: i386-low.c $(i386_low_h) $(server_h) $(target_h) > i387-fp.o: i387-fp.c $(server_h) > > linux-low.o: linux-low.c $(linux_low_h) $(linux_ptrace_h) $(linux_procfs_h) \ > - $(server_h) $(linux_osdata_h) > + $(server_h) $(linux_osdata_h) $(agent_h) > $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< @USE_THREAD_DB@ > > linux-arm-low.o: linux-arm-low.c $(linux_low_h) $(server_h) \ > @@ -468,7 +468,7 @@ linux-s390-low.o: linux-s390-low.c $(linux_low_h) $(server_h) > linux-sh-low.o: linux-sh-low.c $(linux_low_h) $(server_h) > linux-tic6x-low.o: linux-tic6x-low.c $(linux_low_h) $(server_h) > linux-x86-low.o: linux-x86-low.c $(linux_low_h) $(server_h) \ > - $(gdb_proc_service_h) $(i386_low_h) > + $(gdb_proc_service_h) $(i386_low_h) $(agent_h) > linux-xtensa-low.o: linux-xtensa-low.c xtensa-xtregs.c $(linux_low_h) $(server_h) > > lynx-low.o: lynx-low.c $(server_h) $(target_h) $(lynx_low_h) > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index c9b8a3b..d9f4b12 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -19,6 +19,7 @@ > #include "server.h" > #include "linux-low.h" > #include "linux-osdata.h" > +#include "agent.h" > > #include > #include > @@ -1320,7 +1321,7 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat) > if ((wstat == NULL > || (WIFSTOPPED (*wstat) && WSTOPSIG (*wstat) != SIGTRAP)) > && supports_fast_tracepoints () > - && in_process_agent_loaded ()) > + && agent_loaded_p ()) But the checks aren't equivalent. in_process_agent_loaded checks for a bunch of libinproctrace.so symbols, agent_loaded_p only checks for a few symbols in common/agent.c. > { > struct fast_tpoint_collect_status status; > int r; > @@ -2251,7 +2252,7 @@ retry: > if (WIFSTOPPED (w) > && WSTOPSIG (w) != SIGTRAP > && supports_fast_tracepoints () > - && in_process_agent_loaded ()) > + && agent_loaded_p ()) > { > if (debug_threads) > fprintf (stderr, > @@ -2795,7 +2796,7 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data) > > /* Allow debugging the jump pad, gdb_collect, etc.. */ > return (supports_fast_tracepoints () > - && in_process_agent_loaded () > + && agent_loaded_p () > && (gdb_breakpoint_here (lwp->stop_pc) > || lwp->stopped_by_watchpoint > || thread->last_resume_kind == resume_step) > diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c > index 365cd52..a662780 100644 > --- a/gdb/gdbserver/linux-x86-low.c > +++ b/gdb/gdbserver/linux-x86-low.c > @@ -28,6 +28,7 @@ > #include "elf/common.h" > > #include "gdb_proc_service.h" > +#include "agent.h" > > /* Defined in auto-generated file i386-linux.c. */ > void init_registers_i386_linux (void); > @@ -1586,7 +1587,7 @@ x86_get_min_fast_tracepoint_insn_len (void) > return 5; > #endif > > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > char errbuf[IPA_BUFSIZ]; > > diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h > index 5c798c8..390f629 100644 > --- a/gdb/gdbserver/server.h > +++ b/gdb/gdbserver/server.h > @@ -436,8 +436,6 @@ char *pfildes (gdb_fildes_t fd); > agent back to GDBserver. */ > #define IPA_BUFSIZ 100 > > -int in_process_agent_loaded (void); > - > void initialize_tracepoint (void); > > extern int tracing; > diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c > index 0e1f9ed..0004ff2 100644 > --- a/gdb/gdbserver/tracepoint.c > +++ b/gdb/gdbserver/tracepoint.c > @@ -228,14 +228,6 @@ static struct > > static struct ipa_sym_addresses ipa_sym_addrs; > > -int all_tracepoint_symbols_looked_up; > - > -int > -in_process_agent_loaded (void) > -{ > - return all_tracepoint_symbols_looked_up; > -} > - > static int read_inferior_integer (CORE_ADDR symaddr, int *val); > > /* Returns true if both the in-process agent library and the static > @@ -247,7 +239,7 @@ in_process_agent_supports_ust (void) > { > int loaded = 0; > > - if (!in_process_agent_loaded ()) > + if (!agent_loaded_p ()) > { > warning ("In-process agent not loaded"); > return 0; > @@ -294,7 +286,7 @@ write_e_ust_not_loaded (char *buffer) > static int > maybe_write_ipa_not_loaded (char *buffer) > { > - if (!in_process_agent_loaded ()) > + if (!agent_loaded_p ()) > { > write_e_ipa_not_loaded (buffer); > return 1; > @@ -309,7 +301,7 @@ maybe_write_ipa_not_loaded (char *buffer) > static int > maybe_write_ipa_ust_not_loaded (char *buffer) > { > - if (!in_process_agent_loaded ()) > + if (!agent_loaded_p ()) > { > write_e_ipa_not_loaded (buffer); > return 1; > @@ -333,7 +325,7 @@ tracepoint_look_up_symbols (void) > { > int i; > > - if (all_tracepoint_symbols_looked_up) > + if (agent_loaded_p ()) > return; > > for (i = 0; i < sizeof (symbol_list) / sizeof (symbol_list[0]); i++) > @@ -349,10 +341,7 @@ tracepoint_look_up_symbols (void) > } > } > > - if (agent_look_up_symbols () != 0) > - return; > - > - all_tracepoint_symbols_looked_up = 1; > + agent_look_up_symbols (); > } > > #endif > @@ -2965,7 +2954,7 @@ install_tracepoint (struct tracepoint *tpoint, char *own_buf) > { > struct tracepoint *tp; > > - if (!in_process_agent_loaded ()) > + if (!agent_loaded_p ()) > { > trace_debug ("Requested a %s tracepoint, but fast " > "tracepoints aren't supported.", > @@ -3041,7 +3030,7 @@ cmd_qtstart (char *packet) > pause_all (1); > > /* Sync the fast tracepoints list in the inferior ftlib. */ > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > download_tracepoints (); > download_trace_state_variables (); > @@ -3142,7 +3131,7 @@ cmd_qtstart (char *packet) > /* Tracing is now active, hits will now start being logged. */ > tracing = 1; > > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > if (write_inferior_integer (ipa_sym_addrs.addr_tracing, 1)) > fatal ("Error setting tracing variable in lib"); > @@ -3201,7 +3190,7 @@ stop_tracing (void) > /* Stop logging. Tracepoints can still be hit, but they will not be > recorded. */ > tracing = 0; > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > if (write_inferior_integer (ipa_sym_addrs.addr_tracing, 0)) > fatal ("Error clearing tracing variable in lib"); > @@ -3249,7 +3238,7 @@ stop_tracing (void) > /* Clear out the tracepoints. */ > clear_installed_tracepoints (); > > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > /* Pull in fast tracepoint trace frames from the inferior lib > buffer into our buffer, even if our buffer is already full, > @@ -3412,7 +3401,7 @@ cmd_qtstatus (char *packet) > trace_debug ("Returning trace status as %d, stop reason %s", > tracing, tracing_stop_reason); > > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > { > pause_all (1); > > @@ -4086,7 +4075,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc) > > /* Pull in fast tracepoint trace frames from the inferior lib buffer into > our buffer. */ > - if (in_process_agent_loaded ()) > + if (agent_loaded_p ()) > upload_fast_traceframes (); > > /* Check if we were indeed collecting data for one of more > @@ -4187,7 +4176,7 @@ handle_tracepoint_bkpts (struct thread_info *tinfo, CORE_ADDR stop_pc) > /* Pull in fast tracepoint trace frames from the inferior in-process > agent's buffer into our buffer. */ > > - if (!in_process_agent_loaded ()) > + if (!agent_loaded_p ()) > return 0; > > upload_fast_traceframes (); Okay, though I'd've preferred if we weren't dropping the in-process qualifier everywhere... Also, "_p" as suffix means "predicate". It's not really necessary if the symbol's name is already clearly a predicate. -- Pedro Alves