Fix py-xmethods.c when compiled with -Werror against Python 2.4

  Does the attached patch fix the issue pointed out by Ulrich Weigand
here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html

ChangeLog
2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>

        * python/py-xmethods.c (invoke_match_method)
        (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
        Cast the second arg to PyObject_GetAttrString and
        PyObject_GetAttrString to char *.
  

> Does the attached patch fix the issue pointed out by Ulrich Weigand
> here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html
> 
> ChangeLog
> 2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>
> 
>         * python/py-xmethods.c (invoke_match_method)
>         (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
>         Cast the second arg to PyObject_GetAttrString and
>         PyObject_GetAttrString to char *.

I can't tell whether it fixes the problem - it should! - but looking
at the patch, I think some lines might have become longer than 80
characters...

Also, it'd be nice to answer Ulrich's question regarding the use
of the constants - whether it might make sense to use the string
directly? Looking at the code, I think that it would be to avoid
duplicating that string? enabled_field_name is only used once,
but then you'd probably use the constant for ... consistency (;-)).

> diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
> index 0062b2d..5ba146f 100644
> --- a/gdb/python/py-xmethods.c
> +++ b/gdb/python/py-xmethods.c
> @@ -106,7 +106,7 @@ invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
>  
>    cleanups = make_cleanup (null_cleanup, NULL);
>  
> -  enabled_field = PyObject_GetAttrString (matcher, enabled_field_name);
> +  enabled_field = PyObject_GetAttrString (matcher, (char *) enabled_field_name);
>    if (enabled_field == NULL)
>      {
>        do_cleanups (cleanups);
> @@ -127,7 +127,7 @@ invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
>        Py_RETURN_NONE;
>      }
>  
> -  match_method = PyObject_GetAttrString (matcher, match_method_name);
> +  match_method = PyObject_GetAttrString (matcher, (char *) match_method_name);
>    if (match_method == NULL)
>      {
>        do_cleanups (cleanups);
> @@ -252,13 +252,13 @@ gdbpy_get_matching_xmethod_workers
>  
>    /* Gather debug method matchers registered globally.  */
>    if (gdb_python_module != NULL
> -      && PyObject_HasAttrString (gdb_python_module, matchers_attr_str))
> +      && PyObject_HasAttrString (gdb_python_module, (char *) matchers_attr_str))
>      {
>        PyObject *gdb_matchers;
>        PyObject *temp = py_xmethod_matcher_list;
>  
>        gdb_matchers = PyObject_GetAttrString (gdb_python_module,
> -					     matchers_attr_str);
> +					     (char *) matchers_attr_str);
>        if (gdb_matchers != NULL)
>  	{
>  	  py_xmethod_matcher_list = PySequence_Concat (temp, gdb_matchers);
> @@ -391,8 +391,8 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang,
>  
>    cleanups = ensure_python_env (get_current_arch (), current_language);
>  
> -  get_arg_types_method =  PyObject_GetAttrString (py_worker,
> -						  get_arg_types_method_name);
> +  get_arg_types_method = PyObject_GetAttrString
> +    (py_worker, (char *) get_arg_types_method_name);
>    if (get_arg_types_method == NULL)
>      {
>        gdbpy_print_stack ();
  

On Wed, Jun 4, 2014 at 11:13 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Does the attached patch fix the issue pointed out by Ulrich Weigand
>> here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html
>>
>> ChangeLog
>> 2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * python/py-xmethods.c (invoke_match_method)
>>         (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
>>         Cast the second arg to PyObject_GetAttrString and
>>         PyObject_GetAttrString to char *.
>
> I can't tell whether it fixes the problem - it should! - but looking
> at the patch, I think some lines might have become longer than 80
> characters...

I double checked again. Two of the lines are at 80. Rest of the lines
touched are less than 80.

> Also, it'd be nice to answer Ulrich's question regarding the use
> of the constants - whether it might make sense to use the string
> directly? Looking at the code, I think that it would be to avoid
> duplicating that string? enabled_field_name is only used once,
> but then you'd probably use the constant for ... consistency (;-)).

Yes. That is the reason. Sorry for not mentioning it earlier.
  

On Wed, Jun 4, 2014 at 11:13 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Does the attached patch fix the issue pointed out by Ulrich Weigand
>> here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html
>>
>> ChangeLog
>> 2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * python/py-xmethods.c (invoke_match_method)
>>         (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
>>         Cast the second arg to PyObject_GetAttrString and
>>         PyObject_GetAttrString to char *.
>
> I can't tell whether it fixes the problem - it should! - but looking
> at the patch, I think some lines might have become longer than 80
> characters...
>
> Also, it'd be nice to answer Ulrich's question regarding the use
> of the constants - whether it might make sense to use the string
> directly? Looking at the code, I think that it would be to avoid
> duplicating that string? enabled_field_name is only used once,
> but then you'd probably use the constant for ... consistency (;-)).

Heh.  Except that this "consistency" exists to work around a problem
that is little documented.
[There's no comments in the code explaining why things are the way they are,
so it's completely not unexpected that someone might come along and think they
could just move those strings into const globals and everything would
be peachy.]

>
>> diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
>> index 0062b2d..5ba146f 100644
>> --- a/gdb/python/py-xmethods.c
>> +++ b/gdb/python/py-xmethods.c
>> @@ -106,7 +106,7 @@ invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
>>
>>    cleanups = make_cleanup (null_cleanup, NULL);
>>
>> -  enabled_field = PyObject_GetAttrString (matcher, enabled_field_name);
>> +  enabled_field = PyObject_GetAttrString (matcher, (char *) enabled_field_name);
>>    if (enabled_field == NULL)
>>      {
>>        do_cleanups (cleanups);
>> @@ -127,7 +127,7 @@ invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
>>        Py_RETURN_NONE;
>>      }
>>
>> -  match_method = PyObject_GetAttrString (matcher, match_method_name);
>> +  match_method = PyObject_GetAttrString (matcher, (char *) match_method_name);
>>    if (match_method == NULL)
>>      {
>>        do_cleanups (cleanups);
>> @@ -252,13 +252,13 @@ gdbpy_get_matching_xmethod_workers
>>
>>    /* Gather debug method matchers registered globally.  */
>>    if (gdb_python_module != NULL
>> -      && PyObject_HasAttrString (gdb_python_module, matchers_attr_str))
>> +      && PyObject_HasAttrString (gdb_python_module, (char *) matchers_attr_str))
>>      {
>>        PyObject *gdb_matchers;
>>        PyObject *temp = py_xmethod_matcher_list;
>>
>>        gdb_matchers = PyObject_GetAttrString (gdb_python_module,
>> -                                          matchers_attr_str);
>> +                                          (char *) matchers_attr_str);
>>        if (gdb_matchers != NULL)
>>       {
>>         py_xmethod_matcher_list = PySequence_Concat (temp, gdb_matchers);
>> @@ -391,8 +391,8 @@ gdbpy_get_xmethod_arg_types (const struct extension_language_defn *extlang,
>>
>>    cleanups = ensure_python_env (get_current_arch (), current_language);
>>
>> -  get_arg_types_method =  PyObject_GetAttrString (py_worker,
>> -                                               get_arg_types_method_name);
>> +  get_arg_types_method = PyObject_GetAttrString
>> +    (py_worker, (char *) get_arg_types_method_name);
>>    if (get_arg_types_method == NULL)
>>      {
>>        gdbpy_print_stack ();

The patch is fine to me, fwiw.
I wouldn't want to litter every instance of the code with comments
explaining the cast.
I think a good place to document the issue is gdb/python/README, fwiw.
  

> > I can't tell whether it fixes the problem - it should! - but looking
> > at the patch, I think some lines might have become longer than 80
> > characters...
> 
> I double checked again. Two of the lines are at 80. Rest of the lines
> touched are less than 80.

Ok - sorry about the noise!
  

On 06/04/2014 07:05 PM, Siva Chandra wrote:
> Does the attached patch fix the issue pointed out by Ulrich Weigand
> here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html
> 
> ChangeLog
> 2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>
> 
>         * python/py-xmethods.c (invoke_match_method)
>         (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
>         Cast the second arg to PyObject_GetAttrString and
>         PyObject_GetAttrString to char *.
> 

How about we handle this in a central place, like Py_DECREF is
handled ?  See python-internal.h.
  

On Wed, Jun 4, 2014 at 12:56 PM, Pedro Alves <palves@redhat.com> wrote:
> On 06/04/2014 07:05 PM, Siva Chandra wrote:
>> Does the attached patch fix the issue pointed out by Ulrich Weigand
>> here: https://sourceware.org/ml/gdb-patches/2014-06/msg00169.html
>>
>> ChangeLog
>> 2014-06-04  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         * python/py-xmethods.c (invoke_match_method)
>>         (gdbpy_get_matching_xmethod_workers, gdbpy_get_xmethod_arg_types):
>>         Cast the second arg to PyObject_GetAttrString and
>>         PyObject_GetAttrString to char *.
>>
>
> How about we handle this in a central place, like Py_DECREF is
> handled ?  See python-internal.h.

This breakage was marked as "nice to have it fixed" for 7.8.
Considering that, do you prefer that we have a Py_DECREF like solution
now, or after 7.8 branching?
  

On 06/04/2014 09:53 PM, Siva Chandra wrote:
> On Wed, Jun 4, 2014 at 12:56 PM, Pedro Alves <palves@redhat.com> wrote:

>> How about we handle this in a central place, like Py_DECREF is
>> handled ?  See python-internal.h.
> 
> This breakage was marked as "nice to have it fixed" for 7.8.
> Considering that, do you prefer that we have a Py_DECREF like solution
> now, or after 7.8 branching?

I think such a solution would be pretty safe.  I'd vote just
doing it now and not having to think about it again.