From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54797 invoked by alias); 18 Sep 2018 15:31:05 -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 51004 invoked by uid 89); 18 Sep 2018 15:31:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=auditing X-HELO: gateway36.websitewelcome.com Received: from gateway36.websitewelcome.com (HELO gateway36.websitewelcome.com) (192.185.197.22) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Sep 2018 15:30:51 +0000 Received: from cm16.websitewelcome.com (cm16.websitewelcome.com [100.42.49.19]) by gateway36.websitewelcome.com (Postfix) with ESMTP id BE13741F2237E for ; Tue, 18 Sep 2018 09:29:31 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id 2Hqvgt7tBaSey2Hr8gJNPP; Tue, 18 Sep 2018 10:24:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=LoRpuwJqBBfjMVsn7tz/GoWpx94hjITy9DNFn1F72cI=; b=laywPgMbz7SjIFO60bpOYBmDhd wbBjVkBpuYtfFxyNIfzdCs99YEYRmvJuP8aJ4Y+7WvIExC4HjwlO4BupgXuA2a8Iqo6bvwAa75sCR 7KrxwLIFYUMTvzbKC7j1BmUVu; Received: from 97-122-190-66.hlrn.qwest.net ([97.122.190.66]:52076 helo=pokyo) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1g2Hqv-0025tE-GV; Tue, 18 Sep 2018 10:23:49 -0500 From: Tom Tromey To: Philippe Waroquiers Cc: gdb-patches@sourceware.org Subject: Re: [RFAv2 1/6] New cli-utils.h/.c function extract_info_print_args References: <20180826165359.1600-1-philippe.waroquiers@skynet.be> <20180826165359.1600-2-philippe.waroquiers@skynet.be> Date: Tue, 18 Sep 2018 15:31:00 -0000 In-Reply-To: <20180826165359.1600-2-philippe.waroquiers@skynet.be> (Philippe Waroquiers's message of "Sun, 26 Aug 2018 18:53:54 +0200") Message-ID: <878t3yu8vv.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-09/txt/msg00635.txt.bz2 >>>>> "Philippe" == Philippe Waroquiers writes: Philippe> cli-utils.c has a new static function extract_arg_maybe_quoted Philippe> that extracts an argument, possibly single quoted. I think it would be better if this handled quotes the same way that gdb_argv (aka libiberty's buildargv) does. That approach seems ok-ish (maybe not perfect), and following it would mean that at least we're not increasing the number of ways that command lines are quoted in gdb. I guess the reason you did not simply use gdb_argv is that the trailing regexp is the rest of the input string. Philippe> +/* See documentation in cli-utils.h. */ Philippe> + Philippe> +void Philippe> +extract_info_print_args (const char* command, Philippe> + const char **args, Philippe> + bool *quiet, Philippe> + std::string *regexp, Philippe> + std::string *t_regexp) I tend to think this should go somewhere else, since cli-utils is more like "low level" parsing helpers. But I don't really know where I guess. Philippe> + if (*args != NULL && **args != '\000') Philippe> + *regexp = extract_arg (args); It seems to me that this should just be "*regexp = args" for backward compatibility. This formulation will stop at whitespace, whereas the previous code did not. If changing the meaning of "info types regexp" where the regexp has whitespace is ok, then switching fully to gdb_argv would be better. I don't know if it is ok to do but I suspect not. Philippe> +/* A helper function to extract an argument from *ARG. An argument is Philippe> + delimited by whitespace, but it can also be optionally quoted using Philippe> + single quote characters. The return value is empty if no argument Philippe> + was found. */ Philippe> + Philippe> +static std::string Philippe> +extract_arg_maybe_quoted (const char **arg) Philippe> +{ One question I have is whether we could just change extract_arg to do the dequoting. Auditing the existing calls to see if this change would negatively impact them might not be too hard. And, it would be nice to make gdb's CLI a bit more regular. Philippe> +#define INFO_PRINT_ARGS_HELP(entity_kind) \ Philippe> +"If NAMEREGEXP is provided, only prints the " entity_kind " whose name\n\ Philippe> +matches NAMEREGEXP.\n\ Philippe> +If -t TYPEREGEXP is provided, only prints the " entity_kind " whose type\n\ Philippe> +matches TYPEREGEXP. Note that the matching is done with the type\n\ Philippe> +printed by the 'whatis' command.\n\ Philippe> +By default, the command might produce headers and/or messages indicating\n\ Philippe> +why no " entity_kind " can be printed.\n\ Philippe> +The flag -q disables the production of these headers and messages." Constructing help text like this is i18n-unfriendly. gdb tries to do this correctly, though IIRC the build isn't all wired up properly and there aren't any actual translations :-/ Maybe it can be reworded somehow. I'm not sure if xgettext can handle this kind of string concatenation in combination with macro expansion, either. Tom