From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9290 invoked by alias); 26 Sep 2012 09:58:41 -0000 Received: (qmail 9281 invoked by uid 22791); 26 Sep 2012 09:58:40 -0000 X-SWARE-Spam-Status: No, hits=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 26 Sep 2012 09:58:24 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8Q9wMf5016748 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 26 Sep 2012 05:58:22 -0400 Received: from spoyarek (spoyarek.pnq.redhat.com [10.65.192.188]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q8Q9wK1A014310; Wed, 26 Sep 2012 05:58:21 -0400 Date: Wed, 26 Sep 2012 09:58:00 -0000 From: Siddhesh Poyarekar To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: [commit][obv] Use TYPE_LENGTH directly where possible Message-ID: <20120926152740.0900e9b7@spoyarek> In-Reply-To: <20120926094227.GA4335@adacore.com> References: <20120926132621.5f45acb7@spoyarek> <20120926094227.GA4335@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-09/txt/msg00571.txt.bz2 On Wed, 26 Sep 2012 11:42:27 +0200, Joel wrote: > > @@ -637,7 +637,7 @@ amd64_return_value (struct gdbarch *gdba > > } > > > > gdb_assert (class[1] != AMD64_MEMORY); > > - gdb_assert (len <= 16); > > + gdb_assert (TYPE_LENGTH (type) <= 16); > > > > for (i = 0; len > 0; i++, len -= 8) > > { > > Why is the type not OK for the assert, and yet OK for the rest of > the code? (the same question applies to other files, as well) This is so that the assert is not subject to any truncation/overflow resulting due to the type of LEN. That way, I don't have to expand LEN since I know that the value is always going to be less than 16 and if something actually goes wrong, then the assert will definitely catch it. > Why is it better to repeat the use of TYPE_LENGTH rather than use > a single variable? It's definitely not obvious to me, and it seems > even simpler to just change the type of variable "len"... This patch > feels like a step backwards, and trying to reduce the size of a patch > would be the wrong justification for it. > It does not make any functional difference at all and the justification is in fact that it reduces the size of the bitpos patch. I have committed similar changes in the past that were deemed to be OK, so I don't see why this patch in particular should be a problem. I have not made changes in places where more than 4-5 substitutions were necessary and where the code started looking unwieldy as a result. I guess both those parameters are subjective, but I couldn't see a coding convention that seems to have been strictly followed throughout the code base. Regards, Siddhesh