From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17500 invoked by alias); 20 Jun 2013 04:59:46 -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 17484 invoked by uid 89); 20 Jun 2013 04:59:45 -0000 X-Spam-SWARE-Status: No, score=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 20 Jun 2013 04:59:44 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5K4xgE1005254 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 20 Jun 2013 00:59:42 -0400 Received: from psique (ovpn-113-144.phx2.redhat.com [10.3.113.144]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r5K4xdI9030156 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 20 Jun 2013 00:59:41 -0400 From: Sergio Durigan Junior To: Wei-cheng Wang Cc: gdb-patches@sourceware.org Subject: Re: [patch] Parse quoted string for restore/dump commands. References: <51B20120.8070909@gmail.com> X-URL: http://www.redhat.com Date: Thu, 20 Jun 2013 05:11:00 -0000 In-Reply-To: (Wei-cheng Wang's message of "Mon, 17 Jun 2013 11:33:11 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-06/txt/msg00532.txt.bz2 On Monday, June 17 2013, Wei-cheng Wang wrote: > Hi Sergio, > > Thank you for you review. > I've fixed them accordingly at the end of the mail. Thanks, Wei. > On Sun, Jun 16, 2013 at 2:37 AM, Sergio Durigan Junior > wrote: >> On Friday, June 07 2013, Wei-cheng Wang wrote: >> >> Thanks for the patch, Wei-cheng. I don't see your name on >> gdb/MAINTAINERS, do you have copyright assignment on file? You will >> need one in order to commit the changes. If you don't have it, please >> contact me off-list and I can send you the forms. > > We've signed the copryright assignment in GDB, on behalf of Andes Technology > in Dec. 2009. Is that sufficent? It is probably fine then, but I am not sure. Some companies have copyright assignments for all of its employees without restrictions, while others make copyright assignments that don't cover everything. A maintainer is the right person to check this. Can some maintainer take a look for Wei, please? >>> >>> - (*cmd) = skip_spaces (*cmd); >>> - end = *cmd + strcspn (*cmd, " \t"); >>> - filename = savestring ((*cmd), end - (*cmd)); >>> + capacity *= 2; >>> + filename = (char *) xrealloc (filename, capacity); >>> + outp = filename + len; >>> + } >>> + } >>> + >>> + *outp = '\0'; >> >> Just for completeness, you could xrealloc (filename, outp - filename + 1) >> to save space. > > I was considering the trade-off between time and space. > Assuming in most cases, there is no space in the path, > the initial size of the buffer is the length to the first delimiter. > If we expand the buffer only 1 more byte a time, xrealloc might be > called much more times than double the size, so I use the same policy > of buildargv. Oh, sorry, I should have been clearer. I was not referring to the xrealloc inside the for/if, but rather I was saying that you could call xrealloc *again* after you set "*outp = '\0'", after the loop finishes. When the loop finishes you might end up with "filename" being larger than you need. > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 7ae0797..a061f27 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,9 @@ > +2013-06-16 Wei-cheng Wang > + Sergio Durigan Junior > + > + * cli/cli-dump.c (scan_filename_with_cleanup): Parse quoted or > + escaped string when scanning filename. > + The rest of the patch looks OK to me now. You will have to wait for a maintainer to (a) say if you're clear to commit the patch (due to the copyright assignment matter), and (b) actually approve the patch. Also, when you commit the patch, please remember to include your info on gdb/MAINTAINERS, since you will be able to commit after approval. Thanks, -- Sergio