This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]