Re: [patch 4/6] Prepare linux_find_memory_regions_full & co. for move

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

On Mon, 01 Apr 2013 21:44:04 +0200, Aleksandar Ristovski wrote:
[...]
> --- a/gdb/target.c
> +++ b/gdb/target.c
[...]
> +static LONGEST
> +target_fileio_read_alloc_1 (const char *filename,
> +			    gdb_byte **buf_p, int padding)
> +{
> +  struct cleanup *close_cleanup;
> +  int fd, target_errno;
> +  void *memory_to_free = NULL;
> +  LONGEST retval;
> +
> +  fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno);
> +  if (fd == -1)
> +    return -1;
> +
> +  close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd);
> +
> +  make_cleanup (free_current_contents, &memory_to_free);
> +  retval = read_alloc (buf_p, fd, target_fileio_read_alloc_1_pread, padding,
> +		       &memory_to_free);


> +  if (retval >= 0)
> +    {
> +      /* Returned allocated memory is interesting for the caller.  */
> +      memory_to_free = NULL;
>      }

BTW ">= 0" is incorrect, the decision of filled in *BUF_P should be "> 0", see
target_fileio_read_stralloc in FSF GDB:
  if (transferred < 0)
    return NULL;
  if (transferred == 0)
    return xstrdup ("");
  bufstr[transferred] = 0;
or even the documentation of target_fileio_read_alloc:
   If a positive value is returned, a
   sufficiently large buffer will be allocated using xmalloc and
   returned in *BUF_P containing the contents of the object.


But I find you wrote a workaround of my bug in the code of read_alloc, the
should have been:
          if (n < 0 || (n == 0 && buf_pos == 0))
	    xfree (buf);
          else
            *buf_p = buf;
	  if (memory_to_free_ptr != NULL)
	    *memory_to_free_ptr = NULL;

Then target_fileio_read_alloc_1 can contain just (if you want):
	gdb_assert (memory_to_free == NULL);


Thanks,
Jan