From: Pedro Alves <palves@redhat.com>
To: Philipp Rudo <prudo@linux.vnet.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 07/18] -Wwrite-strings: Constify work break character arrays
Date: Wed, 05 Apr 2017 13:17:00 -0000 [thread overview]
Message-ID: <93a6610b-eff5-8b82-2310-61a90fa7c270@redhat.com> (raw)
In-Reply-To: <20170405104634.20a5a437@ThinkPad>
On 04/05/2017 09:46 AM, Philipp Rudo wrote:
> Hi Pedro
>
> great series. Just one minor thing I noticed here.
Thanks!
>> /* 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.
Indeed, nice catch. I removed the comment.
And also fixed the typo in the commit's subject: "work" -> "word"...
Here's what I currently have. (Nothing else changed.)
From 9171fb380f5267d2346dd3e6b3c5c80192911dfb Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 5 Apr 2017 12:58:19 +0100
Subject: [PATCH] -Wwrite-strings: Constify word break character arrays
-Wwrite-strings flags several cases of missing casts around
initializations like:
static char *gdb_completer_command_word_break_characters =
" \t\n!@#$%^&*()+=|~`}{[]\"';:?/>.<,";
Obviously these could/should be const. However, while at it, there's
no need for these variables to be pointers instead of arrays. They
are never changed to point to anything else.
Unfortunately, readline's rl_completer_word_break_characters is "char
*", not "const char *". So we always need a cast somewhere. The
approach taken here is to add a new
set_rl_completer_word_break_characters function that becomes the only
place that writes to rl_completer_word_break_characters, and thus the
single place that needs the cast.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* ada-lang.c (ada_completer_word_break_characters): Now a const
array.
(ada_get_gdb_completer_word_break_characters): Constify.
* completer.c (gdb_completer_command_word_break_characters)
(gdb_completer_file_name_break_characters)
(gdb_completer_quote_characters): Now const arrays.
(get_gdb_completer_quote_characters): Constify.
(set_rl_completer_word_break_characters): New function.
(set_gdb_completion_word_break_characters)
(complete_line_internal): Use it.
* completer.h (get_gdb_completer_quote_characters): Constify.
(set_rl_completer_word_break_characters): Declare.
* f-lang.c (f_word_break_characters): Constify.
* language.c (default_word_break_characters): Constify.
* language.h (language_defn::la_word_break_characters): Constify.
(default_word_break_characters): Constify.
* top.c (init_main): Use set_rl_completer_word_break_characters.
---
gdb/ada-lang.c | 6 ++----
gdb/completer.c | 59 ++++++++++++++++++++++++++++++++++-----------------------
gdb/completer.h | 8 +++++++-
gdb/f-lang.c | 2 +-
gdb/language.c | 2 +-
gdb/language.h | 4 ++--
gdb/top.c | 2 +-
7 files changed, 49 insertions(+), 34 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 9b91e0c..7be135e 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -315,9 +315,7 @@ static void ada_free_symbol_cache (struct ada_symbol_cache *sym_cache);
/* Maximum-sized dynamic type. */
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
@@ -558,7 +556,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";
--
2.5.5
next prev parent reply other threads:[~2017-04-05 13:17 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-04 17:25 [PATCH 00/18] gdb: Enable -Wwrite-strings (aka remove -Wno-write-strings) Pedro Alves
2017-04-04 17:26 ` [PATCH 02/18] -Wwrite-strings: Constify macroexp.c:init_shared_buffer Pedro Alves
2017-04-04 17:26 ` [PATCH 05/18] -Wwrite-strings: Constify warning_pre_print Pedro Alves
2017-04-04 17:26 ` [PATCH 08/18] -Wwrite-strings: Constify mi_cmd_argv_ftype's 'command' parameter Pedro Alves
2017-04-04 17:26 ` [PATCH 14/18] -Wwrite-strings: Add a PyArg_ParseTupleAndKeywords "const char *" overload Pedro Alves
2017-04-04 18:37 ` Sergio Durigan Junior
2017-04-05 12:58 ` Pedro Alves
2017-04-05 15:49 ` Sergio Durigan Junior
2017-04-04 17:26 ` [PATCH 01/18] -Wwrite-strings: Constify struct disassemble_info's disassembler_options field Pedro Alves
2017-04-05 7:22 ` Nick Clifton
2017-04-04 17:26 ` [PATCH 09/18] -Wwrite-strings: MI -info-os Pedro Alves
2017-04-04 17:26 ` [PATCH 07/18] -Wwrite-strings: Constify work break character arrays Pedro Alves
2017-04-05 8:46 ` Philipp Rudo
2017-04-05 13:17 ` Pedro Alves [this message]
2017-04-04 17:26 ` [PATCH 15/18] -Wwrite-strings: execute_command calls with string literals Pedro Alves
2017-04-05 7:13 ` Metzger, Markus T
2017-04-05 13:10 ` Pedro Alves
2017-04-04 17:26 ` [PATCH 16/18] -Wwrite-strings: Some constification in gdb/breakpoint.c Pedro Alves
2017-04-04 17:26 ` [PATCH 04/18] -Wwrite-strings: Constify shell_escape and plug make_command leak Pedro Alves
2017-04-04 17:26 ` [PATCH 03/18] -Wwrite-strings: Don't initialize string command variables to empty string Pedro Alves
2017-04-04 17:26 ` [PATCH 06/18] -Wwrite-strings: Constify target_pid_to_str and target_thread_extra_thread_info Pedro Alves
2017-04-04 18:44 ` John Baldwin
2017-04-04 17:31 ` [PATCH 10/18] -Wwrite-strings: gdbserver's 'port' parsing Pedro Alves
2017-04-04 17:32 ` [PATCH 18/18] -Wwrite-strings: Remove -Wno-write-strings Pedro Alves
2019-02-14 16:17 ` Thomas Schwinge
2017-04-04 17:32 ` [PATCH 11/18] -Wwrite-strings: gdbserver/win32-low.c and TARGET_WAITKIND_EXECD Pedro Alves
2017-04-04 17:32 ` [PATCH 13/18] -Wwrite-strings: Wrap PyGetSetDef for construction with string literals Pedro Alves
2017-04-04 18:40 ` Sergio Durigan Junior
2017-04-05 12:35 ` Pedro Alves
2017-04-05 15:48 ` Sergio Durigan Junior
2017-04-05 8:49 ` Philipp Rudo
2017-04-05 13:03 ` Pedro Alves
2017-04-04 17:33 ` [PATCH 12/18] -Wwrite-strings: More fix-old-Python-API wrappers Pedro Alves
2017-04-04 17:36 ` [PATCH 17/18] -Wwrite-strings: The Rest Pedro Alves
2017-04-04 18:44 ` John Baldwin
2017-04-05 12:59 ` Pedro Alves
2017-04-04 18:42 ` [PATCH 00/18] gdb: Enable -Wwrite-strings (aka remove -Wno-write-strings) Sergio Durigan Junior
2017-04-04 19:37 ` Simon Marchi
2017-04-05 18:05 ` Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=93a6610b-eff5-8b82-2310-61a90fa7c270@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=prudo@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).