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

Aleksandar Ristovski
2013-04-02T13:30:08Z

On 13-04-02 09:07 AM, Jan Kratochvil wrote:
> 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);
>
>

Ok, diff from the code at patch 4 (i.e. after applying patches 1-4) that
I will incorporate into patch 4 is pasted below.

I did not put any assert in target_fileio_read_alloc_1 - I don't think
it's useful.

Thanks,

Aleksandar

diff --git a/gdb/target.c b/gdb/target.c
index d8ab056..ed0f3bf 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3522,17 +3522,11 @@ read_alloc (gdb_byte **buf_p, int handle,
read_alloc_pre
if (n <= 0)
{
if (n < 0 || (n == 0 && buf_pos == 0))
- {
- if (memory_to_free_ptr != NULL)
- {
- gdb_assert (buf == *memory_to_free_ptr);
- *memory_to_free_ptr = NULL;
- }
- xfree (buf);
- }
+ xfree (buf);
else
*buf_p = buf;
-
+ if (memory_to_free_ptr != NULL)
+ *memory_to_free_ptr = NULL;
if (n < 0)
{
/* An error occurred. */
@@ -3576,11 +3570,6 @@ target_fileio_read_alloc_1 (const char *filename,
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;
- }
do_cleanups (close_cleanup);
return retval;
}