From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1386 invoked by alias); 11 Jan 2012 15:54:09 -0000 Received: (qmail 1375 invoked by uid 22791); 11 Jan 2012 15:54:08 -0000 X-SWARE-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from router-304.cs.umd.edu (HELO bacon.cs.umd.edu) (128.8.127.145) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 11 Jan 2012 15:53:49 +0000 Received: from wireless-206-196-163-53.umd.edu (wireless-206-196-163-53.umd.edu [206.196.163.53]) (Authenticated sender: khooyp) by bacon.cs.umd.edu (Postfix) with ESMTPSA id CB31DB406D8; Wed, 11 Jan 2012 10:53:43 -0500 (EST) Subject: Re: Make the "python" command resemble the standard Python interpreter Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Khoo Yit Phang In-Reply-To: Date: Wed, 11 Jan 2012 16:04:00 -0000 Cc: Khoo Yit Phang , gdb-patches@sourceware.org Content-Transfer-Encoding: quoted-printable Message-Id: <94906C8E-C23D-4DA3-989D-DDCCFA20FC35@cs.umd.edu> References: To: Kevin Pouget X-CSD-MailScanner-ID: CB31DB406D8.A9918 X-CSD-MailScanner: Found to be clean X-CSD-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-50, required 5, autolearn=not spam, ALL_TRUSTED -50.00) X-CSD-MailScanner-From: khooyp@cs.umd.edu X-CSD-MailScanner-Watermark: 1326902024.15974@B/yOlZtPZYaKS2EDh60QTg 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: 2012-01/txt/msg00353.txt.bz2 Hi, Thanks for the review, I'll attach an updated patch in a moment. I have a f= ew questions: On Jan 11, 2012, at 5:26 AM, Kevin Pouget wrote: >> +static char * >> +gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout, >> + char *prompt) >=20 > I think that 'char *prompt' whould be aligned with FILE *sys_stdin' It is properly tabbed originally and it seems to be the case when I downloa= d the attachment too. Perhaps a email formatting glitch? >> +{ >> + int n; >> + char *p =3D NULL, *p_start, *p_end, *q; >> + volatile struct gdb_exception except; >> + >> + TRY_CATCH (except, RETURN_MASK_ALL) >> + { >> + struct cleanup *cleanup =3D gdbpy_suspend_sigint_handler (); >=20 > new line between declarations and the code Do you mean there should not be a new line? >> + p =3D command_line_input (prompt, 0, "python"); >> + do_cleanups (cleanup); >> + } >=20 > I'm not sure about that, but isn't the clean up supposed to be > executed even if an exception is thrown? it seems not to be the case > here Are you referring to do_cleanups? If I understand correctly, it's to handle= the case where an exception is not thrown (see, e.g., py-value.c). >> + /* Detect Ctrl-C and treat as KeyboardInterrupt. */ >> + if (except.reason =3D=3D RETURN_QUIT) >> + return NULL; >> + >> + /* Handle errors by raising Python exceptions. */ >> + if (except.reason < 0) >> + { >> + /* The thread state is nulled during gdbpy_readline_wrapper, >> + with the original value saved in the following undocumented >> + variable (see Python's Parser/myreadline.c and >> + Modules/readline.c). */ >=20 > comment lines should be aligned with "The thread" (or maybe my tabs > are not displayed properly) That's might be the case. >> +{ >> + Py_InitModule3 ("readline", GdbReadlineMethods, >> + "GDB-provided dummy readline module to prevent conflicts with the sta= ndard readline module."); >=20 > This line is too long, should be < 80 chars I'll shorten the line, but separately, how do you format lines containing s= trings that themselves can be up to 80 chars (e.g., for printing)? Yit January 11, 2012