This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [commit][obv] Use TYPE_LENGTH directly where possible


> @@ -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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]