Re: [PATCH 07/18] -Wwrite-strings: Constify work break character arrays

[Philipp Rudo <prudo at linux dot vnet dot ibm dot com>]

Hi Pedro

great series. Just one minor thing I noticed here.

On Tue,  4 Apr 2017 18:25:40 +0100
Pedro Alves <palves@redhat.com> wrote:
[...]
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 9b91e0c..d31e058 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -317,7 +317,7 @@ static unsigned int varsize_limit;
> 
>  /* FIXME: brobecker/2003-09-17: No longer a const because it is
>     returned by a function that does not return a const char *.  */
> -static char *ada_completer_word_break_characters =
> +static const char ada_completer_word_break_characters[] =
>  #ifdef VMS
>    " \t\n!@#%^&*()+=|~`}{[]\";:?/,-";
>  #else

The comment above doesn't make sense, especially after your
change. I think it should be removed. 

Philipp

> @@ -558,7 +558,7 @@ add_angle_brackets (const char *str)
>    return result;
>  }
> 
> -static char *
> +static const char *
>  ada_get_gdb_completer_word_break_characters (void)
>  {
>    return ada_completer_word_break_characters;
> diff --git a/gdb/completer.c b/gdb/completer.c
> index 45adc62..9183e2a 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -84,29 +84,30 @@ char *line_completion_function (const char *text,
> int matches, readline library sees one in any of the current
> completion strings, it thinks that the string needs to be quoted and
> automatically supplies a leading quote.  */
> -static char *gdb_completer_command_word_break_characters =
> +static const char gdb_completer_command_word_break_characters[] =
>  " \t\n!@#$%^&*()+=|~`}{[]\"';:?/>.<,";
> 
>  /* When completing on file names, we remove from the list of word
>     break characters any characters that are commonly used in file
>     names, such as '-', '+', '~', etc.  Otherwise, readline displays
>     incorrect completion candidates.  */
> -#ifdef HAVE_DOS_BASED_FILE_SYSTEM
>  /* MS-DOS and MS-Windows use colon as part of the drive spec, and
> most programs support @foo style response files.  */
> -static char *gdb_completer_file_name_break_characters = "
> \t\n*|\"';?><@"; +static const char
> gdb_completer_file_name_break_characters[] = +#ifdef
> HAVE_DOS_BASED_FILE_SYSTEM
> +  " \t\n*|\"';?><@";
>  #else
> -static char *gdb_completer_file_name_break_characters = "
> \t\n*|\"';:?><";
> +  " \t\n*|\"';:?><";
>  #endif
> 
>  /* Characters that can be used to quote completion strings.  Note
> that we can't include '"' because the gdb C parser treats such quoted
>     sequences as strings.  */
> -static char *gdb_completer_quote_characters = "'";
> +static const char gdb_completer_quote_characters[] = "'";
>  
>  /* Accessor for some completer data that may interest other files.
> */
> 
> -char *
> +const char *
>  get_gdb_completer_quote_characters (void)
>  {
>    return gdb_completer_quote_characters;
> @@ -652,16 +653,26 @@ expression_completer (struct cmd_list_element
> *ignore, /* See definition in completer.h.  */
> 
>  void
> +set_rl_completer_word_break_characters (const char *break_chars)
> +{
> +  rl_completer_word_break_characters = (char *) break_chars;
> +}
> +
> +/* See definition in completer.h.  */
> +
> +void
>  set_gdb_completion_word_break_characters (completer_ftype *fn)
>  {
> +  const char *break_chars;
> +
>    /* So far we are only interested in differentiating filename
>       completers from everything else.  */
>    if (fn == filename_completer)
> -    rl_completer_word_break_characters
> -      = gdb_completer_file_name_break_characters;
> +    break_chars = gdb_completer_file_name_break_characters;
>    else
> -    rl_completer_word_break_characters
> -      = gdb_completer_command_word_break_characters;
> +    break_chars = gdb_completer_command_word_break_characters;
> +
> +  set_rl_completer_word_break_characters (break_chars);
>  }
> 
>  /* Here are some useful test cases for completion.  FIXME: These
> @@ -743,8 +754,8 @@ complete_line_internal (const char *text,
>       then we will switch to the special word break set for command
>       strings, which leaves out the '-' character used in some
>       commands.  */
> -  rl_completer_word_break_characters =
> -    current_language->la_word_break_characters();
> +  set_rl_completer_word_break_characters
> +    (current_language->la_word_break_characters());
> 
>    /* Decide whether to complete on a list of gdb commands or on
>       symbols.  */
> @@ -821,8 +832,8 @@ complete_line_internal (const char *text,
>  	    }
>  	  /* Ensure that readline does the right thing with respect
> to inserting quotes.  */
> -	  rl_completer_word_break_characters =
> -	    gdb_completer_command_word_break_characters;
> +	  set_rl_completer_word_break_characters
> +	    (gdb_completer_command_word_break_characters);
>  	}
>      }
>    else
> @@ -848,8 +859,8 @@ complete_line_internal (const char *text,
> 
>  		  /* Ensure that readline does the right thing
>  		     with respect to inserting quotes.  */
> -		  rl_completer_word_break_characters =
> -		    gdb_completer_command_word_break_characters;
> +		  set_rl_completer_word_break_characters
> +		    (gdb_completer_command_word_break_characters);
>  		}
>  	      else if (reason == handle_help)
>  		list = NULL;
> @@ -857,8 +868,8 @@ complete_line_internal (const char *text,
>  		{
>  		  if (reason != handle_brkchars)
>  		    list = complete_on_enum (c->enums, p, word);
> -		  rl_completer_word_break_characters =
> -		    gdb_completer_command_word_break_characters;
> +		  set_rl_completer_word_break_characters
> +		    (gdb_completer_command_word_break_characters);
>  		}
>  	      else
>  		{
> @@ -879,8 +890,8 @@ complete_line_internal (const char *text,
>  			     && strchr
> (gdb_completer_file_name_break_characters, p[-1]) == NULL; p--)
>  			;
> -		      rl_completer_word_break_characters =
> -			gdb_completer_file_name_break_characters;
> +		      set_rl_completer_word_break_characters
> +			(gdb_completer_file_name_break_characters);
>  		    }
>  		  if (reason == handle_brkchars
>  		      && c->completer_handle_brkchars != NULL)
> @@ -913,8 +924,8 @@ complete_line_internal (const char *text,
> 
>  	      /* Ensure that readline does the right thing
>  		 with respect to inserting quotes.  */
> -	      rl_completer_word_break_characters =
> -		gdb_completer_command_word_break_characters;
> +	      set_rl_completer_word_break_characters
> +		(gdb_completer_command_word_break_characters);
>  	    }
>  	}
>        else if (reason == handle_help)
> @@ -947,8 +958,8 @@ complete_line_internal (const char *text,
>  				    p[-1]) == NULL;
>  		       p--)
>  		    ;
> -		  rl_completer_word_break_characters =
> -		    gdb_completer_file_name_break_characters;
> +		  set_rl_completer_word_break_characters
> +		    (gdb_completer_file_name_break_characters);
>  		}
>  	      if (reason == handle_brkchars
>  		  && c->completer_handle_brkchars != NULL)
> diff --git a/gdb/completer.h b/gdb/completer.h
> index 416b313..2aa1987 100644
> --- a/gdb/completer.h
> +++ b/gdb/completer.h
> @@ -99,10 +99,16 @@ extern VEC (char_ptr) *reg_or_group_completer
> (struct cmd_list_element *, extern VEC (char_ptr) *reggroup_completer
> (struct cmd_list_element *, const char *, const char *);
> 
> -extern char *get_gdb_completer_quote_characters (void);
> +extern const char *get_gdb_completer_quote_characters (void);
> 
>  extern char *gdb_completion_word_break_characters (void);
> 
> +/* Set the word break characters array to BREAK_CHARS.  This function
> +   is useful as const-correct alternative to direct assignment to
> +   rl_completer_word_break_characters, which is "char *",
> +   not "const char *".  */
> +extern void set_rl_completer_word_break_characters (const char
> *break_chars); +
>  /* Set the word break characters array to the corresponding set of
>     chars, based on FN.  This function is useful for cases when the
>     completer doesn't know the type of the completion until some
> diff --git a/gdb/f-lang.c b/gdb/f-lang.c
> index 8aba5ef..3c30d75 100644
> --- a/gdb/f-lang.c
> +++ b/gdb/f-lang.c
> @@ -203,7 +203,7 @@ f_language_arch_info (struct gdbarch *gdbarch,
> 
>  /* Remove the modules separator :: from the default break list.  */
> 
> -static char *
> +static const char *
>  f_word_break_characters (void)
>  {
>    static char *retval;
> diff --git a/gdb/language.c b/gdb/language.c
> index 119c07e..f1fc220 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -698,7 +698,7 @@ default_pass_by_reference (struct type *type)
>     delimiting words.  This is a reasonable default value that
>     most languages should be able to use.  */
> 
> -char *
> +const char *
>  default_word_break_characters (void)
>  {
>    return " \t\n!@#$%^&*()+=|~`}{[]\"';:?/>.<,-";
> diff --git a/gdb/language.h b/gdb/language.h
> index 3d21e4e..96080ac 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -321,7 +321,7 @@ struct language_defn
>      char string_lower_bound;
> 
>      /* The list of characters forming word boundaries.  */
> -    char *(*la_word_break_characters) (void);
> +    const char *(*la_word_break_characters) (void);
> 
>      /* Should return a vector of all symbols which are possible
>         completions for TEXT.  WORD is the entire command on which the
> @@ -583,7 +583,7 @@ extern char *language_class_name_from_physname
> (const struct language_defn *, const char *physname);
> 
>  /* Splitting strings into words.  */
> -extern char *default_word_break_characters (void);
> +extern const char *default_word_break_characters (void);
> 
>  /* Print the index of an array element using the C99 syntax.  */
>  extern void default_print_array_index (struct value *index_value,
> diff --git a/gdb/top.c b/gdb/top.c
> index 295b680..4a22be1 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -2022,7 +2022,7 @@ init_main (void)
>    /* Setup important stuff for command line editing.  */
>    rl_completion_word_break_hook =
> gdb_completion_word_break_characters; rl_completion_entry_function =
> readline_line_completion_function;
> -  rl_completer_word_break_characters = default_word_break_characters
> ();
> +  set_rl_completer_word_break_characters
> (default_word_break_characters ()); rl_completer_quote_characters =
> get_gdb_completer_quote_characters ();
> rl_completion_display_matches_hook = cli_display_match_list;
> rl_readline_name = "gdb";