[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)
Pedro Alves
palves@redhat.com
Mon Sep 2 14:09:00 GMT 2013
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
More information about the Gdb-patches
mailing list