[PATCH 14/18] -Wwrite-strings: Add a PyArg_ParseTupleAndKeywords "const char *" overload

Sergio Durigan Junior sergiodj@redhat.com
Wed Apr 5 15:49:00 GMT 2017


On Wednesday, April 05 2017, Pedro Alves wrote:

> On 04/04/2017 07:37 PM, Sergio Durigan Junior wrote:
>> On Tuesday, April 04 2017, Pedro Alves wrote:
>
>>> However, to avoid having all callers add the 'const_cast<char **>'
>>> themselves, this commit define a PyArg_ParseTupleAndKeywords overload
>>> here with a corresponding 'keywords' parameter type that does the cast
>>> in a single place.
>>>
>>> As in the PyGetSetDef patch, I'd be fine with naming this
>>> gdb_PyArg_ParseTupleAndKeywords instead, if people would find having
>>> our own overload confusing.
>> 
>> As much as I like the overloading approach in general, I prefer to see
>> our own overloads marked with the "gdb_", just to make things clearer.
>
> OK, updated patch below.  WDYT?

LGTM.  Thanks!

>> 
>>> BTW, this API issue was discussed years ago in the python-dev list:
>>>
>>>  https://mail.python.org/pipermail/python-dev/2006-February/060689.html
>>>
>>> ... and an overload for C++ was suggested in there somewhere.
>>> Unfortunately, it was never added.
>> 
>> Interesting.  I wonder if there's an actual bug/issue opened on their
>> bugzilla about this.
>> 
>> Hm, after a quick search I didn't find anything.  Maybe it's worth
>> creating one.
>
> I had found that thread via this bug: https://bugs.python.org/issue1772673

Oh, I see.  I was searching for the function name only.  Thanks.

> From 2ea02b3a4f5e7e0a1ecfb206d8a5f04b37876bd2 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 5 Apr 2017 13:45:55 +0100
> Subject: [PATCH] -Wwrite-strings: Add a PyArg_ParseTupleAndKeywords "const
>  char *" overload
>
> -Wwrite-strings flags code like:
>
>    static char *keywords[] = {"command", "from_tty", "to_string", NULL };
>
> as needing "(char *)" casts, because string literals are "const char []".
>
> We can get rid of the casts by changing the array type like this:
>
>  -  static char *keywords[] = {"command", "from_tty", "to_string", NULL };
>  +  static const char *keywords[] = {"command", "from_tty", "to_string", NULL };
>
> However, passing the such array to PyArg_ParseTupleAndKeywords no longer
> works OOTB, because PyArg_ParseTupleAndKeywords expects a "char **":
>
>   PyArg_ParseTupleAndKeywords(PyObject *args, PyObject *kw,
>                               const char *format,
> 			      char *keywords[], ...);
>
> and "const char **" is not implicitly convertible to "char **".  C++
> is more tolerant that C here WRT aliasing, and a const_cast<char **>
> is fine.  However, to avoid having all callers do the cast themselves,
> this commit defines a gdb_PyArg_ParseTupleAndKeywords function here
> with a corresponding 'keywords' parameter type that does the cast in a
> single place.
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* python/python-internal.h (gdb_PyArg_ParseTupleAndKeywords): New
> 	static inline function.
> 	* python/py-arch.c (archpy_disassemble): Constify 'keywords'
> 	array and use gdb_PyArg_ParseTupleAndKeywords.
> 	* python/py-cmd.c (cmdpy_init): Likewise.
> 	* python/py-finishbreakpoint.c (bpfinishpy_init): Likewise.
> 	* python/py-inferior.c (infpy_read_memory, infpy_write_memory)
> 	(infpy_search_memory): Likewise.
> 	* python/py-objfile.c (objfpy_add_separate_debug_file)
> 	(gdbpy_lookup_objfile): Likewise.
> 	* python/py-symbol.c (gdbpy_lookup_symbol)
> 	(gdbpy_lookup_global_symbol): Likewise.
> 	* python/py-type.c (gdbpy_lookup_type): Likewise.
> 	* python/py-value.c (valpy_lazy_string, valpy_string): Likewise.
> 	* python/python.c (execute_gdb_command, gdbpy_write, gdbpy_flush):
> 	Likewise.
> ---
>  gdb/python/py-arch.c             |  7 ++++---
>  gdb/python/py-breakpoint.c       | 10 +++++-----
>  gdb/python/py-cmd.c              | 10 +++++-----
>  gdb/python/py-finishbreakpoint.c |  6 +++---
>  gdb/python/py-inferior.c         | 21 ++++++++++-----------
>  gdb/python/py-objfile.c          | 10 +++++-----
>  gdb/python/py-symbol.c           | 13 +++++++------
>  gdb/python/py-type.c             |  6 +++---
>  gdb/python/py-value.c            | 12 ++++++------
>  gdb/python/python-internal.h     | 28 ++++++++++++++++++++++++++++
>  gdb/python/python.c              | 20 ++++++++++----------
>  11 files changed, 86 insertions(+), 57 deletions(-)
>
> diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
> index 71d2989..cfadb47 100644
> --- a/gdb/python/py-arch.c
> +++ b/gdb/python/py-arch.c
> @@ -116,7 +116,7 @@ archpy_name (PyObject *self, PyObject *args)
>  static PyObject *
>  archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
>  {
> -  static char *keywords[] = { "start_pc", "end_pc", "count", NULL };
> +  static const char *keywords[] = { "start_pc", "end_pc", "count", NULL };
>    CORE_ADDR start, end = 0;
>    CORE_ADDR pc;
>    gdb_py_ulongest start_temp;
> @@ -126,8 +126,9 @@ archpy_disassemble (PyObject *self, PyObject *args, PyObject *kw)
>  
>    ARCHPY_REQUIRE_VALID (self, gdbarch);
>  
> -  if (!PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO", keywords,
> -                                    &start_temp, &end_obj, &count_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, GDB_PY_LLU_ARG "|OO",
> +					keywords, &start_temp, &end_obj,
> +					&count_obj))
>      return NULL;
>  
>    start = start_temp;
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index 34f46fb..a08501e 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -637,8 +637,8 @@ bppy_get_ignore_count (PyObject *self, void *closure)
>  static int
>  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>  {
> -  static char *keywords[] = { "spec", "type", "wp_class", "internal",
> -			      "temporary", NULL };
> +  static const char *keywords[] = { "spec", "type", "wp_class", "internal",
> +				    "temporary", NULL };
>    const char *spec;
>    int type = bp_breakpoint;
>    int access_type = hw_write;
> @@ -647,9 +647,9 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    int internal_bp = 0;
>    int temporary_bp = 0;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiOO", keywords,
> -				     &spec, &type, &access_type,
> -				     &internal, &temporary))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "s|iiOO", keywords,
> +					&spec, &type, &access_type,
> +					&internal, &temporary))
>      return -1;
>  
>    if (internal)
> diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
> index 3aaf7f9..3e60673 100644
> --- a/gdb/python/py-cmd.c
> +++ b/gdb/python/py-cmd.c
> @@ -490,8 +490,8 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
>    char *docstring = NULL;
>    struct cmd_list_element **cmd_list;
>    char *cmd_name, *pfx_name;
> -  static char *keywords[] = { "name", "command_class", "completer_class",
> -			      "prefix", NULL };
> +  static const char *keywords[] = { "name", "command_class", "completer_class",
> +				    "prefix", NULL };
>    PyObject *is_prefix = NULL;
>    int cmp;
>  
> @@ -504,9 +504,9 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
>        return -1;
>      }
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "si|iO",
> -				     keywords, &name, &cmdtype,
> -			  &completetype, &is_prefix))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "si|iO",
> +					keywords, &name, &cmdtype,
> +					&completetype, &is_prefix))
>      return -1;
>  
>    if (cmdtype != no_class && cmdtype != class_run
> diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
> index 76189b8..06f6ff9 100644
> --- a/gdb/python/py-finishbreakpoint.c
> +++ b/gdb/python/py-finishbreakpoint.c
> @@ -156,7 +156,7 @@ bpfinishpy_post_stop_hook (struct gdbpy_breakpoint_object *bp_obj)
>  static int
>  bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>  {
> -  static char *keywords[] = { "frame", "internal", NULL };
> +  static const char *keywords[] = { "frame", "internal", NULL };
>    struct finish_breakpoint_object *self_bpfinish =
>        (struct finish_breakpoint_object *) self;
>    PyObject *frame_obj = NULL;
> @@ -169,8 +169,8 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>    CORE_ADDR pc;
>    struct symbol *function;
>  
> -  if (!PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
> -                                    &frame_obj, &internal))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|OO", keywords,
> +					&frame_obj, &internal))
>      return -1;
>  
>    TRY
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index 77fc543..41bedb5 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -435,10 +435,10 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
>    CORE_ADDR addr, length;
>    gdb_byte *buffer = NULL;
>    PyObject *addr_obj, *length_obj, *result;
> -  static char *keywords[] = { "address", "length", NULL };
> +  static const char *keywords[] = { "address", "length", NULL };
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "OO", keywords,
> -				     &addr_obj, &length_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "OO", keywords,
> +					&addr_obj, &length_obj))
>      return NULL;
>  
>    if (get_addr_from_python (addr_obj, &addr) < 0
> @@ -494,13 +494,12 @@ infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
>    const gdb_byte *buffer;
>    CORE_ADDR addr, length;
>    PyObject *addr_obj, *length_obj = NULL;
> -  static char *keywords[] = { "address", "buffer", "length", NULL };
> +  static const char *keywords[] = { "address", "buffer", "length", NULL };
>  #ifdef IS_PY3K
>    Py_buffer pybuf;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "Os*|O", keywords,
> -				     &addr_obj, &pybuf,
> -				     &length_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "Os*|O", keywords,
> +					&addr_obj, &pybuf, &length_obj))
>      return NULL;
>  
>    buffer = (const gdb_byte *) pybuf.buf;
> @@ -643,7 +642,7 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
>  {
>    struct gdb_exception except = exception_none;
>    CORE_ADDR start_addr, length;
> -  static char *keywords[] = { "address", "length", "pattern", NULL };
> +  static const char *keywords[] = { "address", "length", "pattern", NULL };
>    PyObject *start_addr_obj, *length_obj;
>    Py_ssize_t pattern_size;
>    const gdb_byte *buffer;
> @@ -652,9 +651,9 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
>  #ifdef IS_PY3K
>    Py_buffer pybuf;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "OOs*", keywords,
> -				     &start_addr_obj, &length_obj,
> -				     &pybuf))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "OOs*", keywords,
> +					&start_addr_obj, &length_obj,
> +					&pybuf))
>      return NULL;
>  
>    buffer = (const gdb_byte *) pybuf.buf;
> diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
> index 6a47c17..52f2e1f 100644
> --- a/gdb/python/py-objfile.c
> +++ b/gdb/python/py-objfile.c
> @@ -432,13 +432,13 @@ objfpy_is_valid (PyObject *self, PyObject *args)
>  static PyObject *
>  objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
>  {
> -  static char *keywords[] = { "file_name", NULL };
> +  static const char *keywords[] = { "file_name", NULL };
>    objfile_object *obj = (objfile_object *) self;
>    const char *file_name;
>  
>    OBJFPY_REQUIRE_VALID (obj);
>  
> -  if (!PyArg_ParseTupleAndKeywords (args, kw, "s", keywords, &file_name))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s", keywords, &file_name))
>      return NULL;
>  
>    TRY
> @@ -559,14 +559,14 @@ objfpy_lookup_objfile_by_build_id (const char *build_id)
>  PyObject *
>  gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw)
>  {
> -  static char *keywords[] = { "name", "by_build_id", NULL };
> +  static const char *keywords[] = { "name", "by_build_id", NULL };
>    const char *name;
>    PyObject *by_build_id_obj = NULL;
>    int by_build_id;
>    struct objfile *objfile;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!", keywords,
> -				     &name, &PyBool_Type, &by_build_id_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!", keywords,
> +					&name, &PyBool_Type, &by_build_id_obj))
>      return NULL;
>  
>    by_build_id = 0;
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index 05b002f..c9f83a3 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -373,13 +373,14 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
>    int domain = VAR_DOMAIN;
>    struct field_of_this_result is_a_field_of_this;
>    const char *name;
> -  static char *keywords[] = { "name", "block", "domain", NULL };
> +  static const char *keywords[] = { "name", "block", "domain", NULL };
>    struct symbol *symbol = NULL;
>    PyObject *block_obj = NULL, *sym_obj, *bool_obj;
>    const struct block *block = NULL;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
> -				     &block_object_type, &block_obj, &domain))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
> +					&block_object_type, &block_obj,
> +					&domain))
>      return NULL;
>  
>    if (block_obj)
> @@ -443,12 +444,12 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
>  {
>    int domain = VAR_DOMAIN;
>    const char *name;
> -  static char *keywords[] = { "name", "domain", NULL };
> +  static const char *keywords[] = { "name", "domain", NULL };
>    struct symbol *symbol = NULL;
>    PyObject *sym_obj;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> -				     &domain))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
> +					&domain))
>      return NULL;
>  
>    TRY
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 12b6310..aa20d4c 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -1347,14 +1347,14 @@ type_object_to_type (PyObject *obj)
>  PyObject *
>  gdbpy_lookup_type (PyObject *self, PyObject *args, PyObject *kw)
>  {
> -  static char *keywords[] = { "name", "block", NULL };
> +  static const char *keywords[] = { "name", "block", NULL };
>    const char *type_name = NULL;
>    struct type *type = NULL;
>    PyObject *block_obj = NULL;
>    const struct block *block = NULL;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O", keywords,
> -				     &type_name, &block_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O", keywords,
> +					&type_name, &block_obj))
>      return NULL;
>  
>    if (block_obj)
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index 9c0470f..b5a0ef8 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -431,11 +431,11 @@ valpy_lazy_string (PyObject *self, PyObject *args, PyObject *kw)
>    gdb_py_longest length = -1;
>    struct value *value = ((value_object *) self)->value;
>    const char *user_encoding = NULL;
> -  static char *keywords[] = { "encoding", "length", NULL };
> +  static const char *keywords[] = { "encoding", "length", NULL };
>    PyObject *str_obj = NULL;
>  
> -  if (!PyArg_ParseTupleAndKeywords (args, kw, "|s" GDB_PY_LL_ARG, keywords,
> -				    &user_encoding, &length))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "|s" GDB_PY_LL_ARG,
> +					keywords, &user_encoding, &length))
>      return NULL;
>  
>    if (length < -1)
> @@ -526,10 +526,10 @@ valpy_string (PyObject *self, PyObject *args, PyObject *kw)
>    const char *user_encoding = NULL;
>    const char *la_encoding = NULL;
>    struct type *char_type;
> -  static char *keywords[] = { "encoding", "errors", "length", NULL };
> +  static const char *keywords[] = { "encoding", "errors", "length", NULL };
>  
> -  if (!PyArg_ParseTupleAndKeywords (args, kw, "|ssi", keywords,
> -				    &user_encoding, &errors, &length))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "|ssi", keywords,
> +					&user_encoding, &errors, &length))
>      return NULL;
>  
>    TRY
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> index d4ed170..db75002 100644
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -316,6 +316,34 @@ struct gdb_PyGetSetDef : PyGetSetDef
>    {}
>  };
>  
> +/* The 'keywords' parameter of PyArg_ParseTupleAndKeywords has type
> +   'char **'.  However, string literals are const in C++, and so to
> +   avoid casting at every keyword array definition, we'll need to make
> +   the keywords array an array of 'const char *'.  To avoid having all
> +   callers add a 'const_cast<char **>' themselves when passing such an
> +   array through 'char **', we define our own version of
> +   PyArg_ParseTupleAndKeywords here with a corresponding 'keywords'
> +   parameter type that does the cast in a single place.  (This is not
> +   an overload of PyArg_ParseTupleAndKeywords in order to make it
> +   clearer that we're calling our own function instead of a function
> +   that exists in some newer Python version.)  */
> +
> +static inline int
> +gdb_PyArg_ParseTupleAndKeywords (PyObject *args, PyObject *kw,
> +				 const char *format, const char **keywords, ...)
> +{
> +  va_list ap;
> +  int res;
> +
> +  va_start (ap, keywords);
> +  res = PyArg_VaParseTupleAndKeywords (args, kw, format,
> +				       const_cast<char **> (keywords),
> +				       ap);
> +  va_end (ap);
> +
> +  return res;
> +}
> +
>  /* In order to be able to parse symtab_and_line_to_sal_object function
>     a real symtab_and_line structure is needed.  */
>  #include "symtab.h"
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index a7aff53..25f475f 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -572,11 +572,11 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
>    const char *arg;
>    PyObject *from_tty_obj = NULL, *to_string_obj = NULL;
>    int from_tty, to_string;
> -  static char *keywords[] = {"command", "from_tty", "to_string", NULL };
> +  static const char *keywords[] = { "command", "from_tty", "to_string", NULL };
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|O!O!", keywords, &arg,
> -				     &PyBool_Type, &from_tty_obj,
> -				     &PyBool_Type, &to_string_obj))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!O!", keywords, &arg,
> +					&PyBool_Type, &from_tty_obj,
> +					&PyBool_Type, &to_string_obj))
>      return NULL;
>  
>    from_tty = 0;
> @@ -1047,11 +1047,11 @@ static PyObject *
>  gdbpy_write (PyObject *self, PyObject *args, PyObject *kw)
>  {
>    const char *arg;
> -  static char *keywords[] = {"text", "stream", NULL };
> +  static const char *keywords[] = { "text", "stream", NULL };
>    int stream_type = 0;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &arg,
> -				     &stream_type))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &arg,
> +					&stream_type))
>      return NULL;
>  
>    TRY
> @@ -1088,11 +1088,11 @@ gdbpy_write (PyObject *self, PyObject *args, PyObject *kw)
>  static PyObject *
>  gdbpy_flush (PyObject *self, PyObject *args, PyObject *kw)
>  {
> -  static char *keywords[] = {"stream", NULL };
> +  static const char *keywords[] = { "stream", NULL };
>    int stream_type = 0;
>  
> -  if (! PyArg_ParseTupleAndKeywords (args, kw, "|i", keywords,
> -				     &stream_type))
> +  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "|i", keywords,
> +					&stream_type))
>      return NULL;
>  
>    switch (stream_type)
> -- 
> 2.5.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/



More information about the Gdb-patches mailing list