Re: [python][patch] Inferior and Thread information support.

[Doug Evans <dje at google dot com>]

Hi.  Just a few comments.

On Thu, Jun 17, 2010 at 11:48 PM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> +
> +@findex gdb.read_memory
> +@defmethod Inferior read_memory address length
> +Read @var{length} bytes of memory from the inferior, starting at
> +@var{address}. ?Returns a buffer object, which behaves much like an array
> +or a string. ?It can be modified and given to the @code{gdb.write_memory}
> +function.
> +@end defmethod
> +
> +@findex gdb.write_memory
> +@defmethod Inferior write_memory address buffer @r{[}length@r{]}
> +Write the contents of @var{buffer} to the inferior, starting at
> +@var{address}. ?The @var{buffer} parameter must be a Python object
> +which supports the buffer protocol, i.e., a string, an array or the
> +object returned from @code{gdb.read_memory}. ?If given, @var{length}
> +determines the number of bytes from @var{buffer} to be written.
> +@end defmethod
> +
> +@findex gdb.search_memory
> +@defmethod Inferior search_memory address length pattern @r{[}size@r{]} @r{[}max_count@r{]}
> +Search a region of the inferior memory starting at @var{address} with the
> +given @var{length}. ?@var{pattern} can be a string, a byte array, a buffer
> +object, a number, a @code{gdb.Value} object (@pxref{Values From
> +Inferior}), or a Python list or a tuple with elements in any combination
> +of those types. ?If @var{pattern} is a list or a tuple, each element
> +will be extracted and concatenated together to form @var{pattern}.
> +
> +If @var{size} is given and is non-zero, it specifies the size in bytes
> +of a Python scalar or @code{gdb.Value} in the search pattern. ?If
> +@var{size} is zero or not specified, the size is taken from the value's type
> +in the current language. ?If @var{pattern} is a tuple or list, these
> +actions apply to each element in the tuple or list. ?This is useful
> +when one wants to specify the search pattern as a mixture of types.
> +Note that this means, for example, that in the case of C-like
> +languages a search for an untyped 0x42 will search for @samp{(int)
> +0x42} which is typically four bytes. ?@var{max_count} is the highest
> +number of matches to search for.
> +@end defmethod
> +@end table

I think search_memory should take just the same type of parameter for
the search pattern that write_memory takes: a byte array (etc.).  Keep
the API simple and let the caller do the conversion to a byte array
(or buffer protocol).
We could provide utilities to convert to/from byte arrays (then
write_memory can use them too!) but Python probably has them already.
Then you can punt on the `size' parameter completely.

You also want to document what the result is.

> +@defivar InferiorThread ptid
> +ID of the thread, as assigned by the operating system. ?This attribute is a
> +tuple containing three integers. ?The first is the Process ID (PID); the second
> +is the Lightweight Process ID (TID), and the third is the Thread ID (TID).
> +Either the TID or TID may be 0, which indicates that the operating system
> +does not ?use that identifier.
> +@end defivar

A couple of typos here: TID is duplicated.

> +# Test max-count with size, with different parameter position
> +
> +gdb_test "py print gdb.inferiors()\[0\].search_memory (start_addr, length, \[0x61, 0x61\], 1, 1)" \
> + ?"${one_pattern_found}" "size = 1, max_count = 1"
> +
> +gdb_test "py print gdb.inferiors()\[0\].search_memory (start_addr, length, \[0x61, 0x61\], 1, 2)" \
> + ?"${two_patterns_found}" "size = 1, max_count = 2, normal ordering"
> +
> +gdb_test "py print gdb.inferiors()\[0\].search_memory (start_addr, length, \[0x61, 0x61\], size = 1, max_count = 2)" \
> + ?"${two_patterns_found}" "size = 1, max_count = 2, normal ordering, with keywords"
> +
> +gdb_test "py print gdb.inferiors()\[0\].search_memory (start_addr, length, \[0x61, 0x61\], max_count = 2, size = 1)" \
> + ?"${two_patterns_found}" "size = 1, max_count = 2, inverted ordering"
> +
> +gdb_test "py print gdb.inferiors()\[0\].search_memory (start_addr, length, \['a', 'a'\], max_count = 2)" \
> + ?"${two_patterns_found}" "max_count = 2, with keyword"

The CLI testcase for this tests the parameter ordering in order to
test the parsing of the parameters, but you don't need to do that here
(we can trust python on this).  Doesn't hurt though.