Re: [PATCH 4/8] Prepare linux_find_memory_regions_full & co. for move

Jan Kratochvil
2013-04-14T14:16:08Z

On Tue, 09 Apr 2013 17:27:41 +0200, Aleksandar Ristovski wrote:
[...]
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -3476,55 +3476,67 @@ target_fileio_close_cleanup (void *opaque)
> target_fileio_close (fd, &target_errno);
> }
>
> +/* Helper for target_fileio_read_alloc_1 to make it interruptible. */

Here should be added (reason: so that one can easily find all the
read_alloc_pread_ftype instances):

static read_alloc_pread_ftype target_fileio_read_alloc_1_pread;

(And move the read_alloc_pread_ftype definition above.)

> +
> +static int
> +target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
> + ULONGEST offset, int *target_errno)
> +{
> + QUIT;
> +
> + return target_fileio_pread (handle, read_buf, len, offset, target_errno);
> +}
> +
> /* Read target file FILENAME. Store the result in *BUF_P and
> return the size of the transferred data. PADDING additional bytes are
> available in *BUF_P. This is a helper function for
> target_fileio_read_alloc; see the declaration of that function for more
> information. */
>
> +typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
> + ULONGEST offset, int *target_errno);

Use tabs.

> +
> static LONGEST
> -target_fileio_read_alloc_1 (const char *filename,
> - gdb_byte **buf_p, int padding)
> +read_alloc (gdb_byte **buf_p, int handle, read_alloc_pread_ftype *pread_func,
> + int padding, void **memory_to_free_ptr)
> {
> - struct cleanup *close_cleanup;
> size_t buf_alloc, buf_pos;
> gdb_byte *buf;
> LONGEST n;
> - int fd;
> int target_errno;
>
> - 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);
> -
> /* Start by reading up to 4K at a time. The target will throttle
> this number down if necessary. */
> buf_alloc = 4096;
> buf = xmalloc (buf_alloc);
> + if (memory_to_free_ptr != NULL)
> + {
> + gdb_assert (*memory_to_free_ptr == NULL);
> + *memory_to_free_ptr = buf;
> + }
> buf_pos = 0;
> while (1)
> {
> - n = target_fileio_pread (fd, &buf[buf_pos],
> - buf_alloc - buf_pos - padding, buf_pos,
> - &target_errno);
> - if (n < 0)
> + n = pread_func (handle, &buf[buf_pos], buf_alloc - buf_pos - padding,
> + buf_pos, &target_errno);
> + if (n <= 0)
> {
> - /* An error occurred. */
> - do_cleanups (close_cleanup);
> - xfree (buf);
> - return -1;
> - }
> - else if (n == 0)
> - {
> - /* Read all there was. */
> - do_cleanups (close_cleanup);
> - if (buf_pos == 0)
> + if (n < 0 || (n == 0 && buf_pos == 0))
> xfree (buf);
> else
> *buf_p = buf;
> - return buf_pos;
> + if (memory_to_free_ptr != NULL)
> + *memory_to_free_ptr = NULL;
> + if (n < 0)
> + {
> + /* An error occurred. */
> + return -1;
> + }
> + else
> + {
> + /* Read all there was. */
> + return buf_pos;
> + }
> }
>
> buf_pos += n;
> @@ -3534,12 +3546,34 @@ target_fileio_read_alloc_1 (const char *filename,
> {
> buf_alloc *= 2;
> buf = xrealloc (buf, buf_alloc);
> + if (memory_to_free_ptr != NULL)
> + *memory_to_free_ptr = buf;
> }
> -
> - QUIT;
> }
> }
>

Here could be (so that one can easily find all the read_stralloc_func_ftype
instances):

static read_stralloc_func_ftype target_fileio_read_alloc_1;

(And move the read_stralloc_func_ftype definition above.)

> +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);
> + do_cleanups (close_cleanup);
> + return retval;
> +}
> +
> /* Read target file FILENAME. Store the result in *BUF_P and return
> the size of the transferred data. See the declaration in "target.h"
> function for more information about the return value. */
[...]

Thanks,
Jan