[RFA] Implement -break-commands
Vladimir Prus
vladimir@codesourcery.com
Thu Jul 30 07:54:00 GMT 2009
On Monday 27 July 2009 Tom Tromey wrote:
> >>>>> "Volodya" == Vladimir Prus <vladimir@codesourcery.com> writes:
>
> This all looks reasonable to me. I have a few style nits.
>
> Volodya> -/* Read one line from the input stream. If the command is an "end",
> Volodya> - return such an indication to the caller. If PARSE_COMMANDS is true,
> Volodya> - strip leading whitespace (trailing whitespace is always stripped)
> Volodya> - in the line, attempt to recognize GDB control commands, and also
> Volodya> - return an indication if the command is an "else" or a nop.
> Volodya> - Otherwise, only "end" is recognized. */
>
> Volodya> -static enum misc_command_type
> Volodya> -read_next_line (struct command_line **command, int parse_commands)
> Volodya> +char *
> Volodya> +read_next_line ()
>
> This should be marked `static' (it is declared that way but it is
> clearer to mark the definition as well). I think it needs a header
> comment.
>
> Volodya> static enum command_control_type
> Volodya> -recurse_read_control_structure (struct command_line *current_cmd)
> Volodya> +recurse_read_control_structure (char * (*read_next_line_func) (),
> Volodya> + struct command_line *current_cmd)
>
> The header comment should be updated to mention the new argument.
> And if you don't mind, please fix the reference to the non-existing
> "parent_control" parameter.
>
> Volodya> +extern struct command_line *read_command_lines_1
> Volodya> +(char * (*read_next_line_func) (), int parse_commands);
>
> Indentation on the 2nd line.
>
> There's some other little style nits in the patch -- over-bracing in the
> second patch, mostly.
>
> Volodya> +void
> Volodya> +breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
>
> Needs a header comment.
>
> Volodya> +static char **mi_command_line_array;
> Volodya> +static int mi_command_line_array_cnt;
> Volodya> +static int mi_command_line_array_ptr;
>
> [...]
>
> Volodya> + break_command = read_command_lines_1 (mi_read_next_line, 0);
>
> I think this would be cleaner if read_command_lines_1 took a "user_data"
> argument and then there were no new globals.
This revision addresses all the comments above, except that request for 'closure' pointer to
read_command_lines_1 if addressed by a comment saying why we don't do that. Eli, I've
also applied your documentation suggestions. Is this OK?
- Volodya
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1.diff
Type: text/x-patch
Size: 7583 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20090730/dad6b384/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2.diff
Type: text/x-patch
Size: 11343 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20090730/dad6b384/attachment-0001.bin>
More information about the Gdb-patches
mailing list