From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id A14F03858D28 for ; Sat, 30 Mar 2024 23:48:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A14F03858D28 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=lancelotsix.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lancelotsix.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A14F03858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=51.195.220.111 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711842496; cv=none; b=oHpiS01WhblKM/bN/TEjoXPSzoJrnkw2QhntHn5Y0sLw2F2hMgY43GY8WAwWfc4smzzUg+IoIUezpgXV+BbGLBKYSJhfy1koXmLA+KA7pvCCDmVsw1AkappgpD/SyQyKpIFusegjq1RG/7Mx8acZv/Rz/Ur8gm9s5XccCs3B7GM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711842496; c=relaxed/simple; bh=j4Bel4easdH3Y0Ud9O2YLZ2H85hs0soWs4eXGmErE6w=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=E2GNJrn6Ia5yk4nlzArKaR1RYOGYvA0pJoeURDb61J6cVTHuGiEvpb91taWf2pyUAIKqOPFJS5j1rPiyjayiccDtY3CLM1KdWkZJzbQK8EP4KY9iFzrOJYkNrof0dy00hIlw1mjph0ozfWiwzwnPnClcHEGefewI/VXOuTEpadA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from octopus (cust120-dsl54.idnet.net [212.69.54.120]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 91E8580B16; Sat, 30 Mar 2024 23:48:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=lancelotsix.com; s=2021; t=1711842493; bh=j4Bel4easdH3Y0Ud9O2YLZ2H85hs0soWs4eXGmErE6w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=koLR6EW8op0YcjEoMe3WQbe3CWDAi8PVBGgwAWD9kHL7ThSrDTbp7twoaVMvTG0GC oMCb2yatds3WfoTOSH1DCbXycZyKfzF9DL93aCUb1fFo7beUEw+Grul4QHtVVg15rb Z5V/N9IsDhqc/Fb1ilB2pB4NtpZmq1GDegWiT8S5jOIMKoN6+E6531Gn5eqQootBIs jmRk22dcir69lAWNEX3oF8fOB6Ku47b0K4VqCRAGUDFZ9+cNrFiCmKt44CU1rTqDrk a+Q7J8WAd+a1FCkILwtaFEIWBG4+OB2+VTwKJPp1HwunV2KDMPHpzL2TX9KPUedgKA 7z1DqwRqvgPGg== Date: Sat, 30 Mar 2024 23:48:07 +0000 From: Lancelot SIX To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/6] gdb: improve escaping when completing filenames Message-ID: <20240330234807.y2qwpjwn2kaa6vtd@octopus> References: <0543026b57b831d545d99e325d6e6c3ed9c4c968.1711712401.git.aburgess@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <0543026b57b831d545d99e325d6e6c3ed9c4c968.1711712401.git.aburgess@redhat.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.6.2 (lndn.lancelotsix.com [0.0.0.0]); Sat, 30 Mar 2024 23:48:13 +0000 (UTC) X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Andrew, Thanks for working on that! I have a couple of really minor comments below. On Fri, Mar 29, 2024 at 11:42:27AM +0000, Andrew Burgess wrote: > --- a/gdb/completer.c > +++ b/gdb/completer.c > @@ -204,6 +204,153 @@ noop_completer (struct cmd_list_element *ignore, > { > } > > +/* Return 1 if the character at EINDEX in STRING is quoted (there is an > + unclosed quoted string), or if the character at EINDEX is quoted by a > + backslash. */ > + > +static int > +gdb_completer_file_name_char_is_quoted (char *string, int eindex) > +{ > + for (int i = 0; i <= eindex && string[i] != '\0'; ) > + { > + char c = string[i]; > + > + if (c == '\\') > + { > + /* The backslash itself is not quoted. */ > + if (i >= eindex) > + return 0; > + ++i; > + /* But the next character is. */ > + if (i >= eindex) > + return 1; > + if (string[i] == '\0') > + return 0; > + ++i; > + continue; > + } > + else if (strchr (rl_completer_quote_characters, c) != nullptr) > + { > + /* This assumes that extract_string_maybe_quoted can handle a > + string quoted with character C. Currently this is true as the > + only characters we put in rl_completer_quote_characters are > + single and/or double quotes, both of which > + extract_string_maybe_quoted can handle. */ Maybe it is worth asserting that assumption gdb_assert (c == '"' || c == '\''); > + const char *tmp = &string[i]; > + (void) extract_string_maybe_quoted (&tmp); > + i = tmp - string; > + > + /* Consider any character within the string we just skipped over > + as quoted, though this might not be completely correct; the > + opening and closing quotes are not themselves quoted. But so > + far this doesn't seem to have caused any issues. */ > + if (i >= eindex) > + return 1; > + } > + else > + ++i; > + } > + > + return 0; > +} > + > +/* Removing character escaping from FILENAME. QUOTE_CHAR is the quote > + character around FILENAME or the null-character if there is no quoting > + around FILENAME. */ > + > +static char * > +gdb_completer_file_name_dequote (char *filename, int quote_char) > +{ > + std::string tmp; > + > + if (quote_char == '\'') > + { > + /* There is no backslash escaping within a single quoted string. In > + this case we can just return the input string. */ > + tmp = filename; > + } > + else if (quote_char == '"') > + { > + /* Remove escaping from a double quoted string. */ > + for (const char *input = filename; > + *input != '\0'; > + ++input) > + { > + if (input[0] == '\\' > + && input[1] != '\0' > + && strchr ("\"\\", input[1]) != nullptr) > + ++input; > + tmp += *input; > + } > + } > + else > + { It might be overkill, but I would probably add "gdb_assert (quote_char == '\0')". > + /* Remove escaping from an unquoted string. */ > + for (const char *input = filename; > + *input != '\0'; > + ++input) > + { > + /* We allow anything to be escaped in an unquoted string. */ > + if (*input == '\\') > + { > + ++input; > + if (*input == '\0') > + break; > + } > + > + tmp += *input; > + } > + } > + > + return strdup (tmp.c_str ()); > +} > + > +/* Apply character escaping to the file name in TEXT. QUOTE_PTR points to > + the quote character surrounding TEXT, or points to the null-character if > + there are no quotes around TEXT. MATCH_TYPE will be one of the readline > + constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or > + many completions. */ > + > +static char * > +gdb_completer_file_name_quote (char *text, int match_type, char *quote_ptr) The match_type argument is never used, maybe mark it with [[maybe_unused]]? > +{ > + std::string str; > + > + if (*quote_ptr == '\'') > + { > + /* There is no backslash escaping permitted within a single quoted > + string, so in this case we can just return the input sting. */ > + str = text; > + } > + else if (*quote_ptr == '"') > + { > + /* Add escaping for a double quoted filename. */ > + for (const char *input = text; > + *input != '\0'; > + ++input) > + { > + if (strchr ("\"\\", *input) != nullptr) > + str += '\\'; > + str += *input; > + } > + } > + else > + { Similarly, I’d add a "gdb_assert (*quote_ptr == '\0')". > + /* Add escaping for an unquoted filename. */ > + for (const char *input = text; > + *input != '\0'; > + ++input) > + { > + if (strchr (" \t\n\\\"'", *input) > + != nullptr) > + str += '\\'; > + str += *input; > + } > + } > + > + return strdup (str.c_str ()); > +} > + Best, Lancelot.