From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20190 invoked by alias); 27 Sep 2012 09:59:25 -0000 Received: (qmail 20182 invoked by uid 22791); 27 Sep 2012 09:59:25 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 27 Sep 2012 09:59:20 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 728FF1C69DC; Thu, 27 Sep 2012 05:59:19 -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 OpPvIT2IhcIX; Thu, 27 Sep 2012 05:59:19 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 408CA1C69A9; Thu, 27 Sep 2012 05:59:19 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 4A0B2C880A; Thu, 27 Sep 2012 11:59:17 +0200 (CEST) Date: Thu, 27 Sep 2012 09:59:00 -0000 From: Joel Brobecker To: Siddhesh Poyarekar Cc: gdb-patches@sourceware.org Subject: Re: [commit][obv] Use TYPE_LENGTH directly where possible Message-ID: <20120927095917.GA9624@adacore.com> References: <20120926132621.5f45acb7@spoyarek> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120926132621.5f45acb7@spoyarek> User-Agent: Mutt/1.5.21 (2010-09-15) 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/msg00617.txt.bz2 Hi Siddhesh, > [1] http://sourceware.org/ml/gdb-cvs/2012-09/msg00147.html > [2] http://sourceware.org/ml/gdb-patches/2012-08/msg00144.html > > * amd64-tdep.c (amd64_return_value): Use TYPE_LENGTH directly. > * bfin-tdep.c (bfin_extract_return_value): Likewise. > (bfin_store_return_value): Likewise. > * cris-tdep.c (cris_store_return_value): Likewise. > (cris_extract_return_value): Likewise. > * h8300-tdep.c (h8300_extract_return_value): Likewise. > * hppa-tdep.c (hppa64_return_value): Likewise. > * lm32-tdep.c (lm32_store_return_value): Likewise. > * microblaze-tdep.c (microblaze_store_return_value): Likewise. > * spu-tdep.c (spu_value_from_register): Likewise. > * vax-tdep.c (vax_return_value): Likewise. Following the discussion that started at: http://www.sourceware.org/ml/gdb-patches/2012-09/msg00570.html Here is what I suggest we do: > Index: gdb/amd64-tdep.c [...] > gdb_assert (class[1] != AMD64_MEMORY); > - gdb_assert (len <= 16); > + gdb_assert (TYPE_LENGTH (type) <= 16); Add a comment here to warn us against replacing the expression with asserting on len, with an explanation as to why. This needs to be done at every location where this trick is used. Do you think it's better to do things this way rather than declare len with the correct type? > Index: gdb/cris-tdep.c > Index: gdb/h8300-tdep.c > Index: gdb/hppa-tdep.c > Index: gdb/lm32-tdep.c > Index: gdb/microblaze-tdep.c > Index: gdb/spu-tdep.c > Index: gdb/vax-tdep.c Revert changes in those files, and declare the associated variable with the correct type. Thank you, -- Joel