From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10797 invoked by alias); 4 Oct 2011 15:51:10 -0000 Received: (qmail 10785 invoked by uid 22791); 4 Oct 2011 15:51:08 -0000 X-SWARE-Spam-Status: No, hits=-7.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 04 Oct 2011 15:50:50 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p94FoljO005889 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 4 Oct 2011 11:50:48 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p94Fol32006257; Tue, 4 Oct 2011 11:50:47 -0400 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p94FokWs016851; Tue, 4 Oct 2011 11:50:46 -0400 From: Tom Tromey To: Abhijit Halder Cc: "gdb-patches\@sourceware.org ml" Subject: Re: [PATCH] PR-10034 Bad space handling in `set remote exec-file' command. References: Date: Tue, 04 Oct 2011 15:51:00 -0000 In-Reply-To: (Abhijit Halder's message of "Sun, 2 Oct 2011 12:51:45 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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: 2011-10/txt/msg00088.txt.bz2 >>>>> "Abhijit" == Abhijit Halder writes: Abhijit> In the `set remote exec-file' command if we provide space at the end Abhijit> of the file-name, the space is not being chopped off and being Abhijit> considered as part of file-name. This behavior is inconsistent across Abhijit> similar set commands like `set logging file' etc. My patch will fix Abhijit> that problem. Please review this patch and put your comments. I can't tell if you re-posted the same patch or if it has changes. Please: * If you are sending a ping, just send a ping, as a followup to the patch being pinged, so that it threads properly in a threading mail reader. * If you are sending a new patch, again send it as a followup, and also indicate what changed and why. This patch doesn't have a test case, but should. Abhijit> case var_string_noescape: Abhijit> - if (arg == NULL) Abhijit> - arg = ""; Abhijit> - if (*(char **) c->var != NULL) Abhijit> - xfree (*(char **) c->var); Abhijit> - *(char **) c->var = xstrdup (arg); Abhijit> - break; Why are you changing this case? Abhijit> case var_optional_filename: Abhijit> if (arg == NULL) Abhijit> arg = ""; Abhijit> + else Abhijit> + { Abhijit> + /* Clear trailing whitespace. */ Abhijit> + char *ptr = arg + strlen (arg) - 1; Abhijit> + Abhijit> + while (ptr >= arg && (*ptr == ' ' || *ptr == '\t')) Abhijit> + ptr--; Abhijit> + *(ptr + 1) = '\0'; Why not use remove_trailing_whitespace? You mentioned it in an earlier note, it seems best to just use it from the start. What happens if the resulting string is empty? At least for var_filename this should give an error. Abhijit> @@ -419,7 +423,7 @@ cmd_show_list (struct cmd_list_element * Abhijit> for (; list != NULL; list = list->next) Abhijit> { Abhijit> /* If we find a prefix, run its list, prefixing our output by its Abhijit> - prefix (with "show " skipped). */ Abhijit> + prefix (with "show " skipped). */ Abhijit> if (list->prefixlist && !list->abbrev_flag) Abhijit> { Abhijit> struct cleanup *optionlist_chain This hunk looks like a gratuitous change. Tom