Re: [patch 5/6] Move linux_find_memory_regions_full & co.

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 22 Mar 2013 14:04:16 +0100, Aleksandar Ristovski wrote:
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -3477,69 +3477,6 @@ target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
[...]
> -read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
> -	    int padding, void **memory_to_free_ptr)
[...]
> -	  buf = xrealloc (buf, buf_alloc);
> -	}

vs.

> --- a/gdb/common/common-utils.c
> +++ b/gdb/common/common-utils.c
> @@ -348,3 +348,95 @@ skip_spaces_const (const char *chp)
[...]
> +read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
> +	    int padding, void **memory_to_free_ptr)
[...]
> +	  buf = xrealloc (buf, buf_alloc);
> +	  if (memory_to_free_ptr != NULL)
> +	    *memory_to_free_ptr = buf;
> +	}

This is not a move, one does not see what has really changed in the diffs.



> -read_stralloc (const char *filename, read_stralloc_func_ftype *func)
> -{
> -  gdb_byte *buffer;
> -  char *bufstr;
> -  LONGEST i, transferred;
> -
> -  transferred = func (filename, &buffer, 1);

vs.

> +read_stralloc (const char *filename, read_stralloc_func_ftype *func)
> +{
> +  char *buffer;
> +  LONGEST i, transferred;
> +
> +  transferred = func (filename, (gdb_byte **) &buffer, 1);

Likewise this is not just a move.


One cannot review it before picking each function and comparing them by hand,
this is why the patchset was split into the change + move parts, but you made
changes in the 'move' parts without updating the 'change' parts.

Please update the patchset.  Going to look at the real code changes of the
last parts.


Thanks,
Jan