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: "Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>
- To: "'Pedro Alves'" <palves at redhat dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Mon, 2 Sep 2013 16:18:02 +0200
- 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> <52249C06 dot 1020100 at redhat dot com>
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : lundi 2 septembre 2013 16:09
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : 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)
>
> 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?
The problem is that TARGET_XFER_E_IO
is only defined in gdb/target.h...
Should I just replace TARGET_XFER_E_IO by -1 and keep the gdb version
otherwise?
Pierre