This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 20 Feb 2014 13:17:04 +0000
- Subject: Re: [PATCH 5/8] Share code on to_xfer_partial for tfile and ctf target
- Authentication-results: sourceware.org; auth=none
- References: <1392185152-21220-1-git-send-email-yao at codesourcery dot com> <1392185152-21220-6-git-send-email-yao at codesourcery dot com>
On 02/12/2014 06:05 AM, Yao Qi wrote:
> +enum target_xfer_status
> +exec_read_only_xfer_partial (gdb_byte *readbuf, ULONGEST offset,
> + ULONGEST len, ULONGEST *xfered_len)
I think 'TARGET _ METHOD _ OVERLOAD_REASON' is clearer and
results in a better interface when we end up with multiple
overloads (thinking ahead). E.g.:
exec_xfer_partial_read_only
exec_xfer_partial_loaded_only
vs:
exec_read_only_xfer_partial
exec_loaded_only_xfer_partial
etc.
Also nicer for discovery with tab completion, IMO.
Another nit is that so far, we've tended to reserve "xfer" for functions
that take both a read and write argument, using "read" and "write"
explicitly in the signature instead in functions that do only
one of the directions. E.g., target_read_partial,
target_write_partial. I think following that results in a
least surpristing API. IOW, I think this function name would
be clearer:
enum target_xfer_status
exec_read_partial_read_only (gdb_byte *readbuf, ULONGEST offset,
ULONGEST len, ULONGEST *xfered_len)
Otherwise looks good to me. Thanks!
--
Pedro Alves