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

Philipp Rudo
2017-04-05T08:46:45Z

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[] = "'";
> \f
> /* 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";