From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29902 invoked by alias); 6 Jan 2010 07:58:17 -0000 Received: (qmail 29893 invoked by uid 22791); 6 Jan 2010 07:58:17 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 06 Jan 2010 07:58:12 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 69F4A2BABBE; Wed, 6 Jan 2010 02:58:10 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Vhz5npOzJJ4R; Wed, 6 Jan 2010 02:58:10 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 2DE462BAB17; Wed, 6 Jan 2010 02:58:09 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 611E3F5936; Wed, 6 Jan 2010 08:57:57 +0100 (CET) Date: Wed, 06 Jan 2010 07:58:00 -0000 From: Joel Brobecker To: Hui Zhu Cc: Stan Shebs , Tom Tromey , Stan Shebs , Michael Snyder , gdb-patches ml Subject: Re: [RFC] Let "gcore" command accept a suffix argument Message-ID: <20100106075757.GF24777@adacore.com> References: <4B29018C.6060307@codesourcery.com> <4B3BEDCC.9040103@earthlink.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2010-01/txt/msg00100.txt.bz2 > 2010-01-06 Hui Zhu > * printcmd.c (ctype.h): New include. > (eval_command): New function. > (_initialize_printcmd): New command "eval". This is looking interesting :). A few comments: * what is the exact semantics of the eval command? I looked at the implementation, and I'm not I understand, or what I understand does not necessarily makes sense to me. So, in plain English first, what is the eval command expected to do, in particular, what parts and how will the command translate in the argument that gets passed. * implementation-wise: > +#define CMDSIZE 1024 > + char cmd[CMDSIZE + 1]; The GNU Coding Standard explicitly recommend against hard-coded arbitrary limitations like these. In particular, your implementation seems to just silently truncate the result if the user uses an argument whose result does not fit in your CMDSIZE. I think the implementation should allow for the argument to grow to any size. > + if (strlen (eval_begin)) > + { > + value = parse_and_eval (eval_begin); > + > + switch (TYPE_CODE (value_type (value))) > + { > + case TYPE_CODE_ARRAY: > + LA_GET_STRING (value, &buffer, &length, > + &char_type, &la_encoding); > + break; > + case TYPE_CODE_INT: > + buffer = plongest (value_as_long (value)); > + length = strlen (buffer); > + break; > + default: > + buffer = eval_begin; > + length = exp - eval_begin; > + break; > + } > + > + if (length > CMDSIZE - (cmdp - cmd)) > + length = CMDSIZE - (cmdp - cmd); > + memcpy (cmdp, buffer, length); > + cmdp += length; I think you should print "value_print" instead of doing the printing yourself. Your implementation is missing a lot of other cases that you need to handle (eg: TYPE_CODE_RANGE, just to mention one). -- Joel