From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20269 invoked by alias); 8 Oct 2013 04:56:27 -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 20260 invoked by uid 89); 8 Oct 2013 04:56:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 08 Oct 2013 04:56:25 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 82BD3116454; Tue, 8 Oct 2013 00:56:44 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id U94ir9ghQ2Mk; Tue, 8 Oct 2013 00:56:44 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 207EF116429; Tue, 8 Oct 2013 00:56:43 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 0F73FE0CCA; Tue, 8 Oct 2013 08:56:20 +0400 (RET) Date: Tue, 08 Oct 2013 04:56:00 -0000 From: Joel Brobecker To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 4/7] Move struct varobj to varobj.h. Message-ID: <20131008045620.GD3092@adacore.com> References: <1379512482-31773-1-git-send-email-yao@codesourcery.com> <1379512482-31773-5-git-send-email-yao@codesourcery.com> <20131002094636.GC2971@adacore.com> <525103F5.90607@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <525103F5.90607@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013-10/txt/msg00205.txt.bz2 > How about this? It does look a lot better to me, FWIW. The only possibly contentious question left would be making struct varobj public, when I personally tend to prefer opaque structures. But I'm fine with this step, as it helps achieve the goal of moving the language-specific stuff out of varobj.c. I think Doug also pretty much agreed to that change. I would give the patch, say, until the end of the week, JIC others want to comment in. How does this new patch affect the rest of the patch series? No effect? If not, we can continue reviewing the remainder. Otherwise, can you post an update? Sorry it's taking so long. I just don't have much time. But as I said, I like the direction this is taking. Thanks! > 2013-10-06 Yao Qi > > * varobj.c (struct varobj): Move most of the fields to > varobj.h. > (struct varobj_dynamic): New struct. > (varobj_get_display_hint) [HAVE_PYTHON]: Adjust. > (varobj_has_more): Likewise. > (dynamic_varobj_has_child_method): Likewise. > (update_dynamic_varobj_children): Likewise. > (varobj_get_num_children): Likewise. > (varobj_list_children, varobj_pretty_printed_p): Likewise. > (install_new_value_visualizer): Likewise. > (install_new_value_visualizer, install_new_value): Likewise. > (varobj_update, new_variable, free_variable): Likewise. > (my_value_of_variable, value_get_print_value): Likewise. > (install_visualizer): Change the type of parameter 'var' to > 'struct varobjd_dynamic *'. Callers update. > * varobj.h (struct varobj): Moved from varobj.c. > (struct varobj) : New field. > @@ -2924,7 +2861,7 @@ value_get_print_value (struct value *value, enum varobj_display_formats format, > #if HAVE_PYTHON > if (gdb_python_initialized) > { > - PyObject *value_formatter = var->pretty_printer; > + PyObject *value_formatter= var->dynamic->pretty_printer; You accidently removed a space before '='. > +/* Every variable in the system has a structure of this type defined > + for it. This structure holds all information necessary to manipulate > + a particular object variable. Members which must be freed are noted. */ > +struct varobj > +{ Not sure if there is a rule for it, or not. But I tend to prefer an empty line between documentation and structure as well (same as with subprograms). Add it if you agree, or else feel free to ignore. This is just an arbitrary preference, AFAIK, and it really does not matter much to me. -- Joel