Re: [patch] Compilation regression with python-2.6 [Re: [PATCH 24/28] introduce gdb_pymodule_addobject]

[Pedro Alves <palves@redhat.com>]

On 05/21/2013 08:58 AM, Jan Kratochvil wrote:

> ./python/py-utils.c: In function ‘gdb_pymodule_addobject’:
> ./python/py-utils.c:445: error: suggest explicit braces to avoid ambiguous ‘else’
> 
> python-devel-2.7.3-13.fc18.x86_64
> /usr/include/python2.7/object.h
> #define Py_DECREF(op)                                   \
>     do {                                                \
>         if (_Py_DEC_REFTOTAL  _Py_REF_DEBUG_COMMA       \
>         --((PyObject*)(op))->ob_refcnt != 0)            \
>             _Py_CHECK_REFCNT(op)                        \
>         else                                            \
>         _Py_Dealloc((PyObject *)(op));                  \
>     } while (0)
> 
> python-devel-2.6.6-36.el6.x86_64
> /usr/include/python2.6/object.h
> #define Py_DECREF(op)                                   \
>     if (_Py_DEC_REFTOTAL  _Py_REF_DEBUG_COMMA           \
>         --((PyObject*)(op))->ob_refcnt != 0)                    \
>         _Py_CHECK_REFCNT(op)                            \
>     else                                                \
>         _Py_Dealloc((PyObject *)(op))
> 


> gdb/
> 2013-05-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Workaround Python 2.6.
> 	* python/py-utils.c (gdb_pymodule_addobject): Wrap Py_DECREF into
> 	a block.
> 
> diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
> index d87eb8c..23fcf3f 100644
> --- a/gdb/python/py-utils.c
> +++ b/gdb/python/py-utils.c
> @@ -443,6 +443,9 @@ gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object)
>    Py_INCREF (object);
>    result = PyModule_AddObject (module, name, object);
>    if (result < 0)
> -    Py_DECREF (object);
> +    {
> +      /* Python 2.6 did not wrap Py_DECREF in do { } while (0);.  */
> +      Py_DECREF (object);
> +    }
>    return result;
>  }
> 

How about we handle this in a central place with something like:

static inline void
gdb_Py_DECREF (void *op)
{
  Py_DECREF (op);
}

#undef Py_DECREF
#define Py_DECREF(op) gdb_Py_DECREF (op)

in gdb/python/python-internal.h?  Then code that uses the macro wouldn't need to care.
I went ahead and did a quick attempt at it.  I checked the gdb.python/ tests all still passed for me.

WDYT?

---

 gdb/python/py-utils.c        |   11 ++---------
 gdb/python/python-internal.h |   12 ++++++++++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index e78dee0..80bacf7 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -31,12 +31,8 @@ py_decref (void *p)
 {
   PyObject *py = p;

-  /* Note that we need the extra braces in this 'if' to avoid a
-     warning from gcc.  */
   if (py)
-    {
-      Py_DECREF (py);
-    }
+    Py_DECREF (py);
 }

 /* Return a new cleanup which will decrement the Python object's
@@ -443,9 +439,6 @@ gdb_pymodule_addobject (PyObject *module, const char *name, PyObject *object)
   Py_INCREF (object);
   result = PyModule_AddObject (module, name, object);
   if (result < 0)
-    {
-      /* Python 2.6 did not wrap Py_DECREF in do { } while (0);.  */
-      Py_DECREF (object);
-    }
+    Py_DECREF (object);
   return result;
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index b01efa1..b5c34b6 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -169,6 +169,18 @@ typedef unsigned long gdb_py_ulongest;

 #endif /* HAVE_LONG_LONG */

+/* Python 2.6 did not wrap Py_DECREF in 'do {...} while (0)', leading
+   to 'suggest explicit braces to avoid ambiguous ‘else’' gcc errors.
+   Wrap it ourselves, so that callers don't need to care.  */
+
+static inline void
+gdb_Py_DECREF (void *op)
+{
+  Py_DECREF (op);
+}
+
+#undef Py_DECREF
+#define Py_DECREF(op) gdb_Py_DECREF (op)

 /* In order to be able to parse symtab_and_line_to_sal_object function
    a real symtab_and_line structure is needed.  */