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

[Khoo Yit Phang <khooyp@cs.umd.edu>]

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