Re: [RFC][python] Fixes and improvements to gdb.Value.

[Eli Zaretskii <eliz@gnu.org>]

> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Date: Sat, 03 Jan 2009 00:24:59 -0200
> 
> This patch has fixes and improvements made to the value python bindings
> that have been made in the python branch since this code has been
> committed to CVS. Ok?

Thanks.

> gdb/doc/
> 2009-01-03  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.texinfo (Basic Python): Document gdb.history.

This part is approved, but I have a few comments:

> +@findex gdb.history

The text below this does not mention "gdb.history" at all.  Should it?
I could imagine a reader who gets here by following the "gdb.history"
index entry, and is then puzzled by not finding that text anywhere.

> +If @var{number} is less than or equal to zero, then @value{GDBN} will
> +take the absolute value of @var{number} and count backward from the
> +last element to find the value to return.

For clarity, I'd separate the zero case and the negative case.  First,
"is less than or equal to zero" is a mouthful that would be eliminated
then; you could simply say "negative".  And second, "count zero
elements backward" is an abstraction that is a better avoided.

>                                             If there is no such element
> +in the value history, an exception will be raised.

Should we say which exception will be raised, and perhaps have a
cross-reference to where exceptions raised in Python code are
described?

> +The return value is always an instance of @code{gdb.Value}
> +(@pxref{Values From Inferior}).
> +@end defun

I'd modify this thusly:

  If no exception is raised, the return value is always an instance of
  @code{gdb.Value} (@pxref{Values From Inferior}).

The point is that we should be crystal clear that a value is returned
only if there's no exception (since there are languages where an
exception, too, can return a value), and that "always" does not
include the case with an exception.

> +   string object converted to a named charset.  If an error occurs during
>     the conversion, NULL will be returned and a python exception will be set.
>  
>     The caller is responsible for xfree'ing the string.  */
> -char *
> -unicode_to_target_string (PyObject *unicode_str)
> +static char *
> +unicode_to_encoded_string (PyObject *unicode_str, const char *charset)

I think our convention is to up-case references to arguments in
comments that describe the function.  So please use "named CHARSET" in
the above comment.

> +/* Converts a python string (8-bit or unicode) to a target string in

Is 8-bit and Unicode the only 2 alternatives here?  IOW, doesn;t this
support Far Eastern multibyte encodings, such as ISO-2022, Big-5,
etc.?  Sorry if I'm talking nonsense out of ignorance about Python's
support of non-ASCII characters.