Re: [RFA 6/8] Use function_view in cli-script.c

[Pedro Alves <palves@redhat.com>]

On 04/19/2018 08:15 PM, Tom Tromey wrote:

> -/* The mi_read_next_line consults these variable to return successive
> -   command lines.  While it would be clearer to use a closure pointer,
> -   it is not expected that any future code will use read_command_lines_1,
> -   therefore no point of overengineering.  */

Ahahahah :-)


>  void
>  mi_cmd_break_commands (const char *command, char **argv, int argc)
>  {
> @@ -509,15 +492,24 @@ mi_cmd_break_commands (const char *command, char **argv, int argc)
>    if (b == NULL)
>      error (_("breakpoint %d not found."), bnum);
>  
> -  mi_command_line_array = argv;
> -  mi_command_line_array_ptr = 1;
> -  mi_command_line_array_cnt = argc;
> +  int count = 1;
> +  gdb::function_view<const char * ()> reader
> +    = [&] ()
> +      {
> +	const char *result = nullptr;
> +	if (count < argc)
> +	  result = argv[count++];
> +	return result;
> +      };

This is incorrect -- The 'reader' function_view is storing an address
to a closure whose lifetime ends right after the function_view ctor
is run.  This a similar issue with assigning a string_view to
a temporary std::string, like:

  std::string_view view = std::string ("temp");

C++ doesn't yet have a mechanism that allows making string_view and
types like it extend the lifetime of temporaries they're bound to,
like const references can.

You can fix the issue at hand here easily by not using function_view
at all:

  auto reader = [&] ()
    {
      const char *result = nullptr;
      if (count < argc)
        result = argv[count++];
      return result;
    };

Now "reader" is the lambda closure itself.

> -	       cmd = read_command_lines (str.c_str (), from_tty, 1,
> -					 (is_tracepoint (b)
> -					  ? check_tracepoint_command : 0),
> -					 b);
> +	       gdb::function_view<void (const char *)> validator;
> +	       if (is_tracepoint (b))
> +		 validator = [=] (const char *line)
> +		   {
> +		     validate_actionline (line, b);
> +		   };
> +
> +	       cmd = read_command_lines (str.c_str (), from_tty, 1, validator);
>  	     }
>  	 }

Similar problem here.  Something like this would fix it:

               auto validator = [=] (const char *line)
                 {
                   validate_actionline (line, b);
                 };
               decltype (callback) null_callback;

	       cmd = read_command_lines (str.c_str (), from_tty, 1, 
                                         is_tracepoint (b) 
                                         ? validator : null_callback);

or use if/else:

    if (is_tracepoint (b))
      {
        cmd = read_command_lines (str.c_str (), from_tty, 1, 
                                  [=] (const char *line)
          {
            validate_actionline (line, b);
          };
      }
    else
     cmd = read_command_lines (str.c_str (), from_tty, 1, nullptr);

Thanks,
Pedro Alves