This is the mail archive of the 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: [RFA] Implement -break-commands

On Monday 27 July 2009 Tom Tromey wrote:

> >>>>> "Volodya" == Vladimir Prus <> 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.

Is that something you want fixed? I do prefer bracing everything, since that
is safer in the long run.

> Volodya> +void
> Volodya> +breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
> Needs a header comment.

Do we have a policy if function should be documented in header, or implementation? It
seems many existing files have comments on implementation. 

> 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.

It would be clearer, though I decided not to do that because it's not
likely we'll need another such function, and the use of globals in this
context is ugly, but safe. Shall I refactor?


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