From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5392 invoked by alias); 24 Jan 2012 03:49:03 -0000 Received: (qmail 5382 invoked by uid 22791); 24 Jan 2012 03:49:02 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 Jan 2012 03:48:44 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0O3mgkE016439 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 23 Jan 2012 22:48:42 -0500 Received: from psique ([10.3.112.12]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0O3mahU016931; Mon, 23 Jan 2012 22:48:38 -0500 From: Sergio Durigan Junior To: Philippe Waroquiers Cc: Pedro Alves , "gdb-patches\@sourceware.org ml" Subject: Re: RFA: remote.c : allow long monitor cmds + allow user to C-c References: <1327265676.23561.25.camel@soleil> <4F1D5335.3090705@redhat.com> <1327350423.2215.6.camel@soleil> X-URL: http://www.redhat.com Date: Tue, 24 Jan 2012 04:43:00 -0000 In-Reply-To: <1327350423.2215.6.camel@soleil> (Philippe Waroquiers's message of "Mon, 23 Jan 2012 21:27:03 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes 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 X-SW-Source: 2012-01/txt/msg00804.txt.bz2 Hello Philippe, Thanks for the patch. Do you have copyright assignment? Some formatting nits. On Monday, January 23 2012, Philippe Waroquiers wrote: > On Mon, 2012-01-23 at 12:31 +0000, Pedro Alves wrote: >> Please use "cvs diff -up", or put "diff -up" in your ~/.cvsrc file. > Oops, 2nd trial. > > Index: gdb/ChangeLog > =================================================================== > RCS file: /cvs/src/src/gdb/ChangeLog,v > retrieving revision 1.13761 > diff -u -p -r1.13761 ChangeLog > --- gdb/ChangeLog 20 Jan 2012 10:31:25 -0000 1.13761 > +++ gdb/ChangeLog 23 Jan 2012 20:22:22 -0000 > @@ -1,3 +1,8 @@ > +2012-01-22 Philippe Waroquiers > + > + * remote.c (remote_rcmd): use getpkt_sane to detect timeout > + and continue the loop. Add QUIT statement. First letter shall be uppercase. Two spaces after period, before beginning a new sentence. The date must also be updated if/when you commit. > Index: gdb/remote.c > =================================================================== > RCS file: /cvs/src/src/gdb/remote.c,v > retrieving revision 1.478 > diff -u -p -r1.478 remote.c > --- gdb/remote.c 20 Jan 2012 09:47:32 -0000 1.478 > +++ gdb/remote.c 23 Jan 2012 20:22:24 -0000 > rs->buf[0] = '\0'; > - getpkt (&rs->buf, &rs->buf_size, 0); > + if (getpkt_sane (&rs->buf, &rs->buf_size, 0) == -1) > + { > + /* Timeout. Continue to (try to) read responses. > + This is better than stopping with an error, assuming the stub > + is still executing the (long) monitor command. > + If needed, the user can interrupt gdb using C-c, obtaining > + an effect similar to stop on timeout. */ > + continue; > + } Same formatting issues here in this comment: two spaces after period. I know almost nothing about this part of the code, but as far as I have seen, the patch looks OK. I am not a maintainer, however. -- Sergio