Re: [RFA] new command to search memory

[Daniel Jacobowitz <drow at false dot org>]

On Wed, Apr 16, 2008 at 07:46:48PM -0700, Doug Evans wrote:
> Here's the revised "find" patch.  It includes all requested changes to date.
> 
> Could someone look at target.c:target_search_memory and make sure it's
> DTRT regarding handling targetdebug and falling back on a default?
> Given that we're not using inheritance here, the current code seems
> like the right way to go, but ...

Yes, that looks right.

>  * Watchpoints can now be set on unreadable memory locations, e.g. addresses
> -which will be allocated using malloc later in program execution.
> +  which will be allocated using malloc later in program execution.

I always get confused about this but current style for NEWS puts
continuation lines up against the left margin.

> +      /* If we see a string, parse it ourselves rather than the normal
> +	 handling of downloading it to target memory.  */

This comment is stale I think?

> +/* Utility implementation of searching memory.  */
> +extern int
> +simple_search_memory (struct target_ops* ops,
> +		      CORE_ADDR start_addr, ULONGEST search_space_len,
> +		      const gdb_byte *pattern, ULONGEST pattern_len,
> +		      CORE_ADDR *found_addrp);
> +
> +/* Main entry point for searching memory.  */
> +extern int
> +target_search_memory (CORE_ADDR start_addr, ULONGEST search_space_len,
> +		      const gdb_byte *pattern, ULONGEST pattern_len,
> +		      CORE_ADDR *found_addrp);
> +

Function names only go against the left margin at the definition.

> Index: gdbserver/Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v
> retrieving revision 1.57
> diff -u -p -u -p -r1.57 Makefile.in
> --- gdbserver/Makefile.in	14 Apr 2008 18:04:00 -0000	1.57
> +++ gdbserver/Makefile.in	17 Apr 2008 02:35:24 -0000
> @@ -81,11 +81,15 @@ BFD_CFLAGS = -I$(BFD_DIR) -I$(BFD_SRC)
>  READLINE_DIR = ${srcdir}/../readline
>  READLINE_DEP = $$(READLINE_DIR)
>  
> +# gnulib
> +INCGNULIB = -I$(srcdir)/../gnulib -I../gnulib
> +
>  # All the includes used for CFLAGS and for lint.
>  # -I. for config files.
>  # -I${srcdir} for our headers.
>  # -I$(srcdir)/../regformats for regdef.h.
> -INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../regformats -I$(INCLUDE_DIR)
> +INCLUDE_CFLAGS = -I. -I${srcdir} -I$(srcdir)/../regformats -I$(INCLUDE_DIR) \
> +	$(INCGNULIB)
>  
>  # M{H,T}_CFLAGS, if defined, has host- and target-dependent CFLAGS
>  # from the config/ directory.
> @@ -143,7 +147,7 @@ TAGFILES = $(SOURCES) ${HFILES} ${ALLPAR
>  
>  OBS = inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
>  	utils.o version.o \
> -	mem-break.o hostio.o \
> +	mem-break.o memmem.o hostio.o \
>  	$(XML_BUILTIN) \
>  	$(DEPFILES) $(LIBOBJS)
>  GDBSERVER_LIBS = @GDBSERVER_LIBS@

The gdbserver configure script should already be taking care of all
these.  In particular we don't want to reference ../gnulib in the
objdir since it won't exist if you're just building gdbserver.  And I
know it will add memmem.o, I tested that.

Does that leave just the declaration?  You can stick it somewhere in
gdbserver instead of bringing in all the replacement gnulib headers.

And you were still talking with Eli about docs.  But other than that
I'm happy with this version.  Thanks for your patience.

-- 
Daniel Jacobowitz
CodeSourcery