From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6275 invoked by alias); 26 Sep 2012 09:42:44 -0000 Received: (qmail 6266 invoked by uid 22791); 26 Sep 2012 09:42:44 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO,TW_EG 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; Wed, 26 Sep 2012 09:42:31 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 633231C7C9D; Wed, 26 Sep 2012 05:42:30 -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 q1mYj2btKvw7; Wed, 26 Sep 2012 05:42:30 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 157171C7C43; Wed, 26 Sep 2012 05:42:29 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 327D4C8050; Wed, 26 Sep 2012 11:42:27 +0200 (CEST) Date: Wed, 26 Sep 2012 09:42: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: <20120926094227.GA4335@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/msg00570.txt.bz2 > @@ -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) > Index: gdb/cris-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/cris-tdep.c,v > retrieving revision 1.185 > diff -u -p -r1.185 cris-tdep.c > --- gdb/cris-tdep.c 18 May 2012 21:02:47 -0000 1.185 > +++ gdb/cris-tdep.c 26 Sep 2012 07:45:40 -0000 > @@ -1662,20 +1662,20 @@ cris_store_return_value (struct type *ty > struct gdbarch *gdbarch = get_regcache_arch (regcache); > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > ULONGEST val; > - int len = TYPE_LENGTH (type); > > - if (len <= 4) > + if (TYPE_LENGTH (type) <= 4) > { > /* Put the return value in R10. */ > - val = extract_unsigned_integer (valbuf, len, byte_order); > + val = extract_unsigned_integer (valbuf, TYPE_LENGTH (type), byte_order); > regcache_cooked_write_unsigned (regcache, ARG1_REGNUM, val); > } > - else if (len <= 8) > + else if (TYPE_LENGTH (type) <= 8) > { > /* Put the return value in R10 and R11. */ > val = extract_unsigned_integer (valbuf, 4, byte_order); > regcache_cooked_write_unsigned (regcache, ARG1_REGNUM, val); > - val = extract_unsigned_integer ((char *)valbuf + 4, len - 4, byte_order); > + val = extract_unsigned_integer ((char *)valbuf + 4, > + TYPE_LENGTH (type) - 4, byte_order); > regcache_cooked_write_unsigned (regcache, ARG2_REGNUM, val); > } > else > @@ -1833,21 +1833,21 @@ cris_extract_return_value (struct type * > struct gdbarch *gdbarch = get_regcache_arch (regcache); > enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > ULONGEST val; > - int len = TYPE_LENGTH (type); > > - if (len <= 4) > + if (TYPE_LENGTH (type) <= 4) > { > /* Get the return value from R10. */ > regcache_cooked_read_unsigned (regcache, ARG1_REGNUM, &val); > - store_unsigned_integer (valbuf, len, byte_order, val); > + store_unsigned_integer (valbuf, TYPE_LENGTH (type), byte_order, val); > } > - else if (len <= 8) > + else if (TYPE_LENGTH (type) <= 8) > { > /* Get the return value from R10 and R11. */ > regcache_cooked_read_unsigned (regcache, ARG1_REGNUM, &val); > store_unsigned_integer (valbuf, 4, byte_order, val); > regcache_cooked_read_unsigned (regcache, ARG2_REGNUM, &val); > - store_unsigned_integer ((char *)valbuf + 4, len - 4, byte_order, val); > + store_unsigned_integer ((char *)valbuf + 4, TYPE_LENGTH (type) - 4, > + byte_order, val); > } > else > error (_("cris_extract_return_value: type length too large")); 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. -- Joel