This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] gdbserver/win32-low.c: Check Read/WriteProcessMemory return value (followup to [RFA] windows-nat.c: Handle ERROR_PARTIAL_COPY in windows_xfer_memory function)
- From: Pedro Alves <palves at redhat dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 02 Sep 2013 15:09:10 +0100
- Subject: Re: [RFA] gdbserver/win32-low.c: Check Read/WriteProcessMemory return value (followup to [RFA] windows-nat.c: Handle ERROR_PARTIAL_COPY in windows_xfer_memory function)
- Authentication-results: sourceware.org; auth=none
- References: <5223bb46 dot c6c0420a dot 5a41 dot 008dSMTPIN_ADDED_BROKEN at mx dot google dot com> <52248978 dot 90500 at redhat dot com> <000301cea7dd$17bc4af0$4734e0d0$ at muller@ics-cnrs.unistra.fr> <52249053 dot 6050103 at redhat dot com> <522494dc dot 297a420a dot 6ab0 dot 6047SMTPIN_ADDED_BROKEN at mx dot google dot com> <522497AF dot 8080800 at redhat dot com> <52249a22 dot 42bd420a dot 28f1 dot 722cSMTPIN_ADDED_BROKEN at mx dot google dot com>
On 09/02/2013 03:00 PM, Pierre Muller wrote:
>>> What about this patch,
>>> it still does not allow to really return the number of bytes read or
>>> written,
>>> but at least it checks correctly if the API calls succeeded.
>>
>> No, as long as the read_memory/write_memory interfaces do not
>> support partial transfers, we should only return true if the
>> all of LEN was transferred. Otherwise, things like:
>>
>> static int
>> gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
>> {
>> ...
>> {
>> res = read_inferior_memory (memaddr, myaddr, len);
>> done_accessing_memory ();
>>
>> return res == 0 ? len : -1;
>> }
>> }
>>
>> will behave incorrectly in the ERROR_PARTIAL_COPY scenario...
>
> This is still done in win32_{read/write}_inferior_memory which are the two
> only callers of the static child_xfer_memory function in win32-low.c
> Thus the aim was to narrow the behavior gap between
> windows-nat.c windows_xfer_memory function
> and the win32-low.c child_xfer_memory function,
> without (for now) changing anything to the beghavior of gdbserver,
> as guaranteed by the
> static int
> win32_write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
> int len)
> {
> return child_xfer_memory (memaddr, (char *) myaddr, len, 1, 0) != len;
> }
>
> code...
>
> The only thing I changed is that child_xfer_memory returns the correct
> amount of read/written memory or -1 if an error, other than
> ERRO_PARTIAL_COPY, occurred.
> Thus I think that your answer is missing the intermediate
> win32_{read/write}_inferior_memory level.
>
Ah, indeed.
Why the different styles in gdb's and gdbserver patches, though?
gdb:
> + if (!success && lasterror == ERROR_PARTIAL_COPY && done > 0)
> + return done;
> + else
> + return success ? done : TARGET_XFER_E_IO;
gdbserver:
> + if (success)
> + return done;
> + else
> + {
> + if (lasterror == ERROR_PARTIAL_COPY && done > 0)
> + return done;
> + else
> + return -1;
> }
We should be able to compare the functions and see at
a glance they are almost duplicates. With the different
styles, it's not immediately obvious. Can you make the
gdbserver code look like gdb's?
Thanks,
--
Pedro Alves