RE: [PATCH] linux-record: Simplify with record_mem_at_reg()

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andreas Arnez
> Sent: Tuesday, March 15, 2016 5:51 PM
> To: gdb-patches@sourceware.org
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>
> Subject: [PATCH] linux-record: Simplify with record_mem_at_reg()

Hello Andreas,

I like the cleanup you're doing and other than a few comments on formatting
I'm OK with the patch.

This is outside of my area, though.  You need the approval of a global maintainer.


> @@ -327,9 +333,7 @@ record_linux_system_call (enum gdb_syscall syscall,
>            || tmpulongest == RECORD_PTRACE_PEEKDATA
>            || tmpulongest == RECORD_PTRACE_PEEKUSR)
>          {
> -          regcache_raw_read_unsigned (regcache, tdep->arg4,
> -                                      &tmpulongest);
> -          if (record_full_arch_list_add_mem ((CORE_ADDR) tmpulongest, 4))
> +	  if (record_mem_at_reg (regcache, tdep->arg4, 4))
>              return -1;
>          }

Indentation seems to be broken for this file.  I wouldn't fix it for individual lines,
though.  I'd either fix it for the entire function in a separate patch or preserve
the broken indentation.


> @@ -561,9 +540,8 @@ record_linux_system_call (enum gdb_syscall syscall,
>        break;
> 
>      case gdb_sys_ustat:
> -      regcache_raw_read_unsigned (regcache, tdep->arg2, &tmpulongest);
> -      if (record_full_arch_list_add_mem ((CORE_ADDR) tmpulongest,
> -					 tdep->size_ustat))
> +      if (record_mem_at_reg (regcache, tdep->arg2,
> +			     tdep->size_ustat))

This now fits onto a single line.


> @@ -574,9 +552,8 @@ record_linux_system_call (enum gdb_syscall syscall,
>        break;
> 
>      case gdb_sys_sigaction:
> -      regcache_raw_read_unsigned (regcache, tdep->arg3, &tmpulongest);
> -      if (record_full_arch_list_add_mem ((CORE_ADDR) tmpulongest,
> -					 tdep->size_old_sigaction))
> +      if (record_mem_at_reg (regcache, tdep->arg3,
> +			     tdep->size_old_sigaction))

Same here and at a few other places.


> @@ -693,12 +662,9 @@ record_linux_system_call (enum gdb_syscall syscall,
> 
>      case gdb_sys_readlink:
>        {
> -        ULONGEST len;
> -
> -        regcache_raw_read_unsigned (regcache, tdep->arg2,
> -                                    &tmpulongest);
> -        regcache_raw_read_unsigned (regcache, tdep->arg3, &len);
> -        if (record_full_arch_list_add_mem ((CORE_ADDR) tmpulongest, (int)
> len))
> +        regcache_raw_read_unsigned (regcache, tdep->arg3, &tmpulongest);
> +	if (record_mem_at_reg (regcache, tdep->arg2,
> +			       (int) tmpulongest))
>            return -1;
>        }

Fits onto one line now.  You don't need the block anymore.


Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928