From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11776 invoked by alias); 21 Feb 2008 22:37:51 -0000 Received: (qmail 11767 invoked by uid 22791); 21 Feb 2008 22:37:50 -0000 X-Spam-Check-By: sourceware.org Received: from gv-out-0910.google.com (HELO gv-out-0910.google.com) (216.239.58.186) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 21 Feb 2008 22:37:26 +0000 Received: by gv-out-0910.google.com with SMTP id n40so134882gve.39 for ; Thu, 21 Feb 2008 14:37:24 -0800 (PST) Received: by 10.142.237.20 with SMTP id k20mr8111321wfh.228.1203633442725; Thu, 21 Feb 2008 14:37:22 -0800 (PST) Received: by 10.143.125.5 with HTTP; Thu, 21 Feb 2008 14:37:22 -0800 (PST) Message-ID: Date: Thu, 21 Feb 2008 22:50:00 -0000 From: "Yakov Lerner" To: "Michael Snyder" , gdb-patches@sourceware.org Subject: Re: simlpe patch implements eval command (with printf-like format and args) In-Reply-To: <1203631895.19253.213.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <1203631895.19253.213.camel@localhost.localdomain> 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: 2008-02/txt/msg00353.txt.bz2 On Fri, Feb 22, 2008 at 12:11 AM, Michael Snyder wrote: > > On Thu, 2008-02-21 at 11:20 +0200, Yakov Lerner wrote: > > This simple patch implements eval command with printf-like syntax: > > > > eval "printf-like-format", comma-separated args > > > > The patch is against cvs-checkedout source. Suggestions are welcome. > > Implementation is very simple. > > Not bad at all, for a first contribution. But I suggest > you try to limit the scope a little bit and make it still > simpler -- maybe have it accept only strings, floats and ints > to start -- and see if you can't implement it less intrusively You mean here, to replicate (with cuts) the code of existing printf_command(), and leave existing printf_command() unchanged, correct ? > eg. without taking over the function for another command and > introducing new cross-module infrastructure. Then we can > maybe iteratively improve on it. > > I'll give you some style and structure feedback on this one: > > > > +/* Grow allocated buffer and appens to it, without causing quadratic > > slowdown */ > > "append", of course -- and comments must be punctuated (in this case, > period followed by two spaces". Plus indentation -- consult the GNU > coding standard. ok > > +/* Grow allocated buffer and appens to it, without causing quadratic > > slowdown */ > > as above. ok > > +static void > > +eval_command (char *arg, int from_tty) > > +{ > > + struct cleanup *old_cleanups; > > + char *str = gdb_own_xasprintf (arg); > > + > > + old_cleanups = make_cleanup (free_current_contents, &str); > > + execute_command (str, from_tty); > > + do_cleanups (old_chain); > > +} > > old_chain is undefined. Probably should be "old_cleanups"? ok Thanks Yakov