This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC stub-side break conditions 4/5] Group agent expression-specific code in its own file


On 01/05/2012 02:56 PM, Luis Machado wrote:

> 2012-01-05  Luis Machado  <lgustavo@codesourcery>
> 
> 	gdbserver/
> 	* server.h: Include tracepoint.h.

Doesn't look that this is true.  But indeed the monolithic server.h will
probably be broken up at some point.  Not having automatic dependency
tracking is IMO what tends to produce central headers (because we're
lazy).

> 	(agent_mem_read, agent_get_trace_state_variable_value,
> 	agent_set_trace_state_variable_value,
> 	agent_tsv_read, agent_mem_read_string, get_get_tsv_func_addr,
> 	get_set_tsv_func_addr): New prototypes.
> 

> -mem-break.o: mem-break.c $(server_h)
> +mem-break.o: mem-break.c $(server_h) $(ax_h)

This change doesn't belong in this patch.

> +
> +static void
> +ax_vdebug (const char *fmt, ...)
> +{
> +  char buf[1024];
> +  va_list ap;
> +
> +  va_start (ap, fmt);
> +  vsprintf (buf, fmt, ap);
> +  fprintf (stderr, "gdbserver/ax: %s\n", buf);

Is this called from the IPA?  Is so, you'll want a change similar
to what Yao did recently to tracepoint.c.  We should promote something
like util.c's PREFIX macro, and use it everywhere to fix this once and
for all:

somewhere global:

#ifdef IN_PROCESS_AGENT
#  define PROG "ipa"
#else
#  define PROG "gdbserver"
#endif

and then:

   fprintf (stderr, PROG "/ax: %s\n", buf);

> +  va_end (ap);
> +}
> +
> +#define ax_debug_1(level, fmt, args...)	\
> +  do {						\
> +    if (level <= debug_threads)			\
> +      ax_vdebug ((fmt), ##args);		\
> +  } while (0)
> +
> +#define ax_debug(FMT, args...)		\
> +  ax_debug_1 (1, FMT, ##args)
> +
> +/* Prefix exported symbols, for good citizenship.  All the symbols
> +   that need exporting are defined in this module.  */

The latter sentence was true where the code was copied from, but it is
not true here.

> +#ifdef IN_PROCESS_AGENT
> +# define get_trace_state_variable_value \
> +  gdb_agent_get_trace_state_variable_value
> +# define set_trace_state_variable_value \
> +  gdb_agent_set_trace_state_variable_value
> +#endif
> +
> +/* This enum must exactly match what is documented in
> +   gdb/doc/agentexpr.texi, including all the numerical values.  */
> +

Otherwise looks good.  Feel free to put it in without waiting
for the other patches to be ok.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]