From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3116 invoked by alias); 12 Jun 2014 13:23:33 -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 3103 invoked by uid 89); 12 Jun 2014 13:23:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 12 Jun 2014 13:23:31 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5CDNSoM017910 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 12 Jun 2014 09:23:28 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5CDNQDL027382; Thu, 12 Jun 2014 09:23:27 -0400 Message-ID: <5399A9CE.4040900@redhat.com> Date: Thu, 12 Jun 2014 13:23:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Pierre Langlois , gdb-patches@sourceware.org Subject: Re: [RFC][PATCH v2][PR remote/16896] Invalidate a register in cache when a remote target failed to write it. References: <537E086A.9030803@embecosm.com> In-Reply-To: <537E086A.9030803@embecosm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-06/txt/msg00502.txt.bz2 Hi Pierre, On 05/22/2014 03:23 PM, Pierre Langlois wrote: > Hello all, > > This patch addresses the issue in patch [1] generically as pointed out by > a comment [2]. > > As opposed to invalidating a register in cache in the remote target before > throwing an error, we can do this using a cleanup in regcache_raw_write. This > patch adds routines to add a regcache_invalidate cleanup to the current chain. > We can use this before target_store_registers and discard it after if an error > was not thrown. > > [1] https://sourceware.org/ml/gdb-patches/2014-05/msg00083.html > [2] https://sourceware.org/ml/gdb-patches/2014-05/msg00357.html Based on the commit log / description of the change in v1, could you propose a commit log for this version of the patch, please? > 2014-05-20 Pierre Langlois > > PR remote/16896 > * regcache.c (register_to_invalidate): New structure. Combines a pointer Double space after periods. "structure. Combines". But just "New structure" is enough. The structure's description should be in the code. We usually spell out "struct" in the context part as well. So: * regcache.c (struct register_to_invalidate): New structure. > to a struct regcache and a register number. > (do_register_invalidate): New function. Call regcache_invalidate. > (make_cleanup_regcache_invalidate): New function. Construct a cleanup > for invalidating a register. > (regcache_raw_write): Call make_cleanup_regcache_invalidate Missing period. Also, please make sure ChangeLog entries are indented with a tab instead of spaces. I suggest: * regcache.c (struct register_to_invalidate): New structure. (do_register_invalidate, make_cleanup_regcache_invalidate): New functions. (regcache_raw_write): Call make_cleanup_regcache_invalidate. * regcache.h (make_cleanup_regcache_invalidate): New prototype. > + > +static void > +do_regcache_invalidate (void *data) > +{ > + struct register_to_invalidate *reg = data; > + regcache_invalidate (reg->regcache, reg->regnum); Empty line after declaration. > +} > + > +struct cleanup * > +make_cleanup_regcache_invalidate (struct regcache *regcache, int regnum) As nothing is calling this outside regcache.c, please make it static. > +{ > + struct register_to_invalidate* reg = XNEW (struct register_to_invalidate); > + reg->regcache = regcache; Empty line after declaration. Thanks! -- Pedro Alves