From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42169 invoked by alias); 19 Apr 2017 15:00:56 -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 41874 invoked by uid 89); 19 Apr 2017 15:00:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=capping, life, concluded X-HELO: mail-wm0-f45.google.com Received: from mail-wm0-f45.google.com (HELO mail-wm0-f45.google.com) (74.125.82.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Apr 2017 15:00:50 +0000 Received: by mail-wm0-f45.google.com with SMTP id m123so14381475wma.0 for ; Wed, 19 Apr 2017 08:00:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=Bt9H9c1fYYwcHxLTndpNMULlhD9d5Tol/4ebPod019s=; b=KaLLyAYv1hOMhcEHa4yHfeL0LQZzEvTlR2IQB3oO6Av9c8sKezMHXIGPZNunqG3FFQ xLG4ksIzwWUVSmkvGLcPW566cCYTZPw1ADg0TGwveKNc5kszcF0V4fwAubS9yfd3a+JO nO9CjOJKlghQLXmeVl+PceYzrvd6rkddenSQ6HVKqFq9JRQ5/Szck/XsvArgF2Wz2l98 mc6DT1WR1SPWBebfPnYyR7u6zqIlRBHyYoOR6b1+WplinfhhI70wNkelUvymudfjfAf+ 7QjueDkSdWlCLOoEZUOVVg50fjpAbc33YqSZCqcteanycs/ciLZiGxLCH30hgc8YKXxA ngOQ== X-Gm-Message-State: AN3rC/5eQAUKBL4bKjTneNXo9bsQIx09YaatYaMnRFGc70w1UCkSYsQ9 MYijLzJ8J8JGqg== X-Received: by 10.28.211.210 with SMTP id k201mr3319190wmg.129.1492614050042; Wed, 19 Apr 2017 08:00:50 -0700 (PDT) Received: from E107787-LIN ([194.214.185.158]) by smtp.gmail.com with ESMTPSA id j138sm4149128wmg.10.2017.04.19.08.00.49 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 19 Apr 2017 08:00:49 -0700 (PDT) From: Yao Qi To: Andreas Arnez Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/9] Fix size capping in write_pieced_value References: <1491586736-21296-1-git-send-email-arnez@linux.vnet.ibm.com> <1491586736-21296-3-git-send-email-arnez@linux.vnet.ibm.com> <86r30wafft.fsf@gmail.com> <86k26gahcg.fsf@gmail.com> Date: Wed, 19 Apr 2017 15:00:00 -0000 In-Reply-To: (Andreas Arnez's message of "Wed, 19 Apr 2017 16:35:54 +0200") Message-ID: <86tw5k8msg.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00567.txt.bz2 Andreas Arnez writes: >> Hi Andreas, >> The description to the logic can go to comments, so that we don't need >> to do "git blame/log" to understand the code. > > Right, I'll add some general explanation and a diagram about the various > bits and offsets (as requested below). > > However, most of the commit message explains a specific bug in a piece > of code that won't exist any more. This aspect doesn't make sense to be > included in the comments, I think. > OK, no problem. >>>>> logic in write_pieced_value for handling this is flawed when there are >>>>> actually bits to skip at the beginning of the first piece: it truncat= es >>>>> the piece size towards the end *before* accounting for the skipped bi= ts >>>>> at the beginning instead of the other way around. >>>>> >>>>> Note that the same bug was already found in read_pieced_value and fix= ed >>>>> there (but not in write_pieced_value), see PR 15391. >>>> >>>> Can we share the code in write_pieced_value and read_pieced_value? The >>>> code computing offsets and bits should be shared. >>> >>> Yes. I have another patch (not posted yet) that merges these two >>> functions. I moved that towards the end of the patch series, so the >>> individual fixes can be incremental. >>> >> >> I'd like to merge the code first, then don't need to fix the same >> problem in two functions read_pieced_value and write_pieced_value (your >> patch 4/9 ~ 9/9 touches both two functions). > > Not sure I understand. Do you mean to merge the functions first while > preserving existing logic, including all the bugs and differences? I What I meant is that 1) make two parts identical (but not introducing new bugs) 2) merge the code to a single function, 3) fix the rest of bugs in the single function, > had started along this path and gave up on it, because I found it too > complicated. From that attempt I've concluded that the current approach > is much less error-prone and easier to follow. There may be some complexity I didn't realize. I don't want to make your life harder. If you are comfortable on this approach, fine by me. --=20 Yao (=E9=BD=90=E5=B0=A7)