From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2627 invoked by alias); 29 Sep 2009 23:36:32 -0000 Received: (qmail 2618 invoked by uid 22791); 29 Sep 2009 23:36:31 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx.southnet.co.nz (HELO viper.snap.net.nz) (202.37.101.20) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 29 Sep 2009 23:36:28 +0000 Received: from totara (153.27.255.123.static.snap.net.nz [123.255.27.153]) by viper.snap.net.nz (Postfix) with ESMTP id B92413DA833; Wed, 30 Sep 2009 12:36:25 +1300 (NZDT) Received: by totara (Postfix, from userid 1000) id D2568C167; Wed, 30 Sep 2009 12:36:24 +1300 (NZDT) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <19138.39416.821174.32806@totara.tehura.co.nz> Date: Tue, 29 Sep 2009 23:36:00 -0000 To: Vladimir Prus Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH:gdb/mi] Update a specified list of variable objects In-Reply-To: <200909282032.55055.vladimir@codesourcery.com> References: <19127.27863.973393.962895@totara.tehura.co.nz> <19128.3154.796642.554720@totara.tehura.co.nz> <19128.5244.844945.680565@totara.tehura.co.nz> <200909282032.55055.vladimir@codesourcery.com> From: nickrob@snap.net.nz (Nick Roberts) 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: 2009-09/txt/msg00940.txt.bz2 > > Actually it's probably better to use isdigit like below > > In fact, it's probably even easier than than. Note this code from > mi_cmd_var_create: > > if (strcmp (name, "-") == 0) > { > xfree (name); > name = varobj_gen_name (); > } > else if (!isalpha (*name)) > error (_("mi_cmd_var_create: name of object must begin with a letter")); > > with this in mind, > > > > + print_values = PRINT_NO_VALUES; > > + else if (argc == 2 && isdigit (*argv[0])) > > + { > > + optind = 1; > > + print_values = mi_parse_values_option (argv[0]); > > + } > > can be written like this: > > else if (argc == 2 && (isdigit (*argv[0]) || *argv[0] == '-')) > { > optind = 1; > print_values = mi_parse_values_option (argv[0]); > } "-var-update --all-values var1 var2" doesn't work in this case. I see a couple of shortcomings with my patch too now: "-var-update 0 var1 var2" doesn't work and "-var-update" with no argument givese a segmentation fault. > > else > > - name = (argv[1]); > > + { > > + while (1) > > + { > > + int opt = mi_getopt ("mi_cmd_var_update", > > + argc, argv, opts, &optind, &optarg); > > And this code can be dropped. What do you think? This is a few lines shorter but what if someone wants to add another option? mi_getopt provides an extensible solution. I'm not really worried which approach is used. Since the general idea seems to be acceptable, I'll write some tests and documentation. Then you can code it a different way if you wish and the only requirement will be that the tests pass. -- Nick http://users.snap.net.nz/~nickrob