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

Jan Kratochvil
2013-04-02T13:07:51Z

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