This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Implement -break-commands
- From: Tom Tromey <tromey at redhat dot com>
- To: Vladimir Prus <vladimir at codesourcery dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Mon, 27 Jul 2009 10:40:42 -0600
- Subject: Re: [RFA] Implement -break-commands
- References: <200907271303.13335.vladimir@codesourcery.com>
- Reply-to: tromey at redhat dot com
>>>>> "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.
Tom