From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21970 invoked by alias); 2 Sep 2013 14:18:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 21957 invoked by uid 89); 2 Sep 2013 14:18:13 -0000 Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.222.211) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Sep 2013 14:18:13 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,MSGID_MULTIPLE_AT autolearn=no version=3.3.2 X-HELO: mailhost.u-strasbg.fr Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antispam (Postfix) with ESMTP id 4A79922076B; Mon, 2 Sep 2013 16:18:10 +0200 (CEST) Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id 3B16C220779; Mon, 2 Sep 2013 16:18:10 +0200 (CEST) Received: from md14.u-strasbg.fr (md14.u-strasbg.fr [130.79.200.249]) by mr1.u-strasbg.fr (Postfix) with ESMTP id 1866722076B; Mon, 2 Sep 2013 16:18:08 +0200 (CEST) Received: from ms18.u-strasbg.fr (ms18.u-strasbg.fr [130.79.204.118]) by md14.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id r82EI7u2020217 ; Mon, 2 Sep 2013 16:18:07 +0200 Received: from E6510Muller (gw-ics.u-strasbg.fr [130.79.210.225]) (Authenticated sender: mullerp) by ms18.u-strasbg.fr (Postfix) with ESMTPSA id F1EE11FD88; Mon, 2 Sep 2013 16:18:05 +0200 (CEST) From: "Pierre Muller" To: "'Pedro Alves'" Cc: References: <5223bb46.c6c0420a.5a41.008dSMTPIN_ADDED_BROKEN@mx.google.com> <52248978.90500@redhat.com> <000301cea7dd$17bc4af0$4734e0d0$@muller@ics-cnrs.unistra.fr> <52249053.6050103@redhat.com> <522494dc.297a420a.6ab0.6047SMTPIN_ADDED_BROKEN@mx.google.com> <522497AF.8080800@redhat.com> <52249a22.42bd420a.28f1.722cSMTPIN_ADDED_BROKEN@mx.google.com> <52249C06.1020100@redhat.com> In-Reply-To: <52249C06.1020100@redhat.com> 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) Date: Mon, 02 Sep 2013 14:18:00 -0000 Message-ID: <000001cea7e7$3c931840$b5b948c0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-09/txt/msg00028.txt.bz2 > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Pedro Alves > Envoy=E9=A0: lundi 2 septembre 2013 16:09 > =C0=A0: Pierre Muller > Cc=A0: gdb-patches@sourceware.org > Objet=A0: 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) >=20 > 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 =3D read_inferior_memory (memaddr, myaddr, len); > >> done_accessing_memory (); > >> > >> return res =3D=3D 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 >=20 > > 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) !=3D l= en; > > } > > > > 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. > > >=20 > Ah, indeed. >=20 >=20 > Why the different styles in gdb's and gdbserver patches, though? >=20 > gdb: >=20 > > + if (!success && lasterror =3D=3D ERROR_PARTIAL_COPY && done > 0) > > + return done; > > + else > > + return success ? done : TARGET_XFER_E_IO; >=20 > gdbserver: >=20 > > + if (success) > > + return done; > > + else > > + { > > + if (lasterror =3D=3D ERROR_PARTIAL_COPY && done > 0) > > + return done; > > + else > > + return -1; > > } >=20 > 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=20 is only defined in gdb/target.h... Should I just replace TARGET_XFER_E_IO by -1 and keep the gdb version otherwise? Pierre