From: Junio C Hamano Date: Wed, 07 Aug 2019 20:37:24 GMT Subject: Re: [PATCH 2/3] trace2: trim whitespace in start message in perf target format Message-ID: In-Reply-To: <827ed342-1079-8145-ed78-1b955264226e@jeffhostetler.com> Jeff Hostetler writes: > On 8/1/2019 5:34 PM, Junio C Hamano wrote: >> "Jeff Hostetler via GitGitGadget" writes: >> >>> From: Jeff Hostetler >>> >>> Trim leading/trailing whitespace from the command line >>> printed in the "start" message in the perf target format. >>> >>> We use `sq_quote_argv_pretty()` to format the message >>> and it adds a leading space to the output. Trim that. >> >> strbuf_trim() not just drops a single leading space, but removes >> consecutive spaces from both ends. But the first char after the SP >> comes from the first arg, and it can never be a whitespace (as a >> payload that begins with a whitespace will be quoted, so it will be >> a single quote), and the last char in the buffer would also be >> either a closing single quote (if the last argument ends with a >> whitespace) or a non whitespace "safe" character, so it is safe to >> use strbuf_trim() here. >> >> I wonder if we want to lose the prepending of SP from >> sq_quote_argv_pretty(), though: > > I was wondering about that too, but I didn't want to presume > to know what all of the callers wanted. And didn't want to > slip in such a change last-minute. I do not think this topic/discussion is a place to do so, either. And compensating on the caller's side, like your patch did, is perfectly fine. I was more disturbed by potential double quoting in some codepaths when I did a cursory audit of users of sq_quote_argv_pretty(), and would think that would be of more importance, though.