Re: RFC: new command to search memory

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon, 28 Jan 2008 10:10:44 -0800
> From: "Doug Evans" <dje@google.com>
> 
> Ping ...

Sorry, I managed to miss this somehow.

The patch for the manual is approved (assuming the code patch will be
approved), except for the following gotchas: 

> +@table @code
> +
> +@kindex find
> +@item find @r{[}/@var{size}@r{]} @r{[}/@var{max_count}@r{]} @var{start_addr}, @@@var{len}, @var{val1} @r{[}, @var{val2}, ...@r{]}
> +@itemx find @r{[}/@var{size}@r{]} @r{[}/@var{max_count}@r{]} @var{start_addr}, @var{end_addr}, @var{val1} @r{[}, @var{val2}, ...@r{]}

These two lines are two long, and since they are in @code, TeX will
not wrap them, and they will overflow the page margins in print.  So
please make them shorter somehow.  Here's one idea:

  @item find @r{[}/@var{options} @var{where} @dots{}

and then describe ``options'' and ``where'' separately.

Also, please use @dots{} instead of literal "...", the former looks
better in print.

> +@var{size} specifies how to interpret the search pattern and is
> +'b' for 8-bit values, 'h' for 16-bit values, 'w' for 32-bit values,

Please use @samp{b}, @samp{h} etc.

> +Strings may be specified for search values, quote them normally.

Is this language-dependent?  If not, then ``normally'' doesn't really
cut it; please describe specifically how to quote.

> +The string value is copied into the search pattern byte by byte,
> +regardless of the endianness of the target and the size specification.

This begs the question: what about non-printable characters? are they
supported, and if so, how?

> +The address of the last value found is stored in convenience variable
> +@samp{$numfound}.
> +A count of the number of matches is stored in @samp{$_}.

Your example has this the other way around (and so does the code,
IIUC):

> +(gdb) find &mixed, @@sizeof(mixed), (char) 'c', (short) 0x1234, (int) 0x87654321
> +0x601054 <mixed.1633>
> +1 pattern found
> +(gdb) print $numfound
> +$1 = 1
> +(gdb) print $_
> +$2 = (void *) 0x601054
> +@end smallexample

> +@item qSearch:memory:@var{address};@var{length};@var{search-pattern}
> +@cindex search memory

You already have "@cindex searching memory" where you describe `find',
so this almost identical index entry in a totally different place
would be confusing: the reader will not know which entry is relevant
for them.  I suggest this instead:

  @cindex searching memory, in remote debugging

> +@cindex @samp{qSearch:memory} packet
> +@anchor{qSearch memory}
> +Search LENGTH bytes at ADDRESS for SEARCH-PATTERN.
> +ADDRESS and LENGTH are encoded in hex.
> +SEARCH-PATTERN is a sequence of bytes, hex encoded.
> +
> +Reply:
> +@table @samp
> +@item 0
> +The pattern was not found.
> +@item 1,address
> +The pattern was found at ADDRESS.

This should use @var{length}, @var{address}, instead of LENGTH,
ADDRESS, etc.: in lower-case and in @var.

> Index: gdb/NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.251
> diff -u -p -u -p -r1.251 NEWS
> --- ./gdb/NEWS  5 Jan 2008 21:50:43 -0000       1.251
> +++ ./gdb/NEWS  15 Jan 2008 21:53:23 -0000
> @@ -3,6 +3,17 @@
>
>  *** Changes since GDB 6.7
>
> +* New command
> +
> +find [/size-char] [/max-count] start-address, end-address|@search-space-size,
> +    val1 [, val2, ...]
> +  Search memory for a sequence of bytes.
> +
> +* New remote packet
> +
> +qSearch:memory:
> +  Search memory for a sequence of bytes.
> +
>  * Change in command line behavior -- corefiles vs. process ids.

This is also fine.

Thanks.