Re: Make the "python" command resemble the standard Python interpreter

Khoo Yit Phang
2012-01-11T15:54:09Z

Hi,

Thanks for the review, I'll attach an updated patch in a moment. I have a few 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)
>
> 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 download the attachment too. Perhaps a email formatting glitch?

>> +{
>> + int n;
>> + char *p = NULL, *p_start, *p_end, *q;
>> + volatile struct gdb_exception except;
>> +
>> + TRY_CATCH (except, RETURN_MASK_ALL)
>> + {
>> + struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
>
> new line between declarations and the code

Do you mean there should not be a new line?

>> + p = command_line_input (prompt, 0, "python");
>> + do_cleanups (cleanup);
>> + }
>
> 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 == 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). */
>
> 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 standard readline module.");
>
> This line is too long, should be < 80 chars

I'll shorten the line, but separately, how do you format lines containing strings that themselves can be up to 80 chars (e.g., for printing)?

Yit
January 11, 2012