From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14282 invoked by alias); 3 Jan 2009 09:42:31 -0000 Received: (qmail 14274 invoked by uid 22791); 3 Jan 2009 09:42:30 -0000 X-SWARE-Spam-Status: No, hits=0.4 required=5.0 tests=AWL,BARRACUDA_BRBL,BAYES_00,J_CHICKENPOX_37,J_CHICKENPOX_51,J_CHICKENPOX_53,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout6.012.net.il (HELO mtaout6.012.net.il) (84.95.2.16) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 03 Jan 2009 09:41:33 +0000 Received: from conversion-daemon.i-mtaout6.012.net.il by i-mtaout6.012.net.il (HyperSendmail v2007.08) id <0KCW00I002WP9Q00@i-mtaout6.012.net.il> for gdb-patches@sourceware.org; Sat, 03 Jan 2009 11:44:11 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.127.220.202]) by i-mtaout6.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0KCW005JU31MHAE0@i-mtaout6.012.net.il>; Sat, 03 Jan 2009 11:44:11 +0200 (IST) Date: Sat, 03 Jan 2009 09:42:00 -0000 From: Eli Zaretskii Subject: Re: [RFC][python] Fixes and improvements to gdb.Value. In-reply-to: <1230949500.8380.140.camel@localhost.localdomain> To: Thiago Jung Bauermann Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: References: <1230949500.8380.140.camel@localhost.localdomain> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-01/txt/msg00006.txt.bz2 > From: Thiago Jung Bauermann > 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 > > * 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.