From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7450 invoked by alias); 4 Jun 2014 19:23:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 7439 invoked by uid 89); 4 Jun 2014 19:23:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ve0-f170.google.com Received: from mail-ve0-f170.google.com (HELO mail-ve0-f170.google.com) (209.85.128.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 04 Jun 2014 19:23:16 +0000 Received: by mail-ve0-f170.google.com with SMTP id db11so9342872veb.15 for ; Wed, 04 Jun 2014 12:23:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=gKh40s+SfgTNsBuC4+KUJE1NvUjx7lkyezju3aBhNOs=; b=YNlmhgntc+g6jht817Id+pnP3p6/MKPFrMrZMVvFbGsZq2acsgDn4/t5WGfgbbotMJ 12Qv3CSdLSMF+/o+FfV6nhv/QwKwGecHucS5jmx3jGec5x+mJdYOChcfHnSd5xvgKXDC BfBpZt2ZlW9AnfGtuW/7Vf1p3hHVJW5W9z95R4+xf8809AbIDWiQOvrVKKaaLAxMnKpx Y4+4nBNgZKDup0geu0a74kCeIEb3V45F0/thibOQUqm49NWesKE0Tg7QwOV8haPM8hxY PuFjaKbc4sSUcwygx1FEQTioth1uPIe5rzBM/CUu3Rb8H9pBnkoaVWmYBHJJk1bcDz4s fVjQ== X-Gm-Message-State: ALoCoQlOQIxC5m2HiBjm0lY5osb4icQ0sO024cE/kz2EW3Yn4TUVleJCtwNe8LhJX1hkLloq7gQx MIME-Version: 1.0 X-Received: by 10.58.128.229 with SMTP id nr5mr4343125veb.76.1401909794446; Wed, 04 Jun 2014 12:23:14 -0700 (PDT) Received: by 10.52.28.230 with HTTP; Wed, 4 Jun 2014 12:23:14 -0700 (PDT) In-Reply-To: <20140604181342.GW4289@adacore.com> References: <20140604181342.GW4289@adacore.com> Date: Wed, 04 Jun 2014 19:23:00 -0000 Message-ID: Subject: Re: [PATCH] Fix py-xmethods.c when compiled with -Werror against Python 2.4 From: Doug Evans To: Joel Brobecker Cc: Siva Chandra , gdb-patches , Ulrich Weigand Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00208.txt.bz2 On Wed, Jun 4, 2014 at 11:13 AM, Joel Brobecker 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 >> >> * 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.