Re: [PATCH v4 1/7] quote: remove leading space in sq_dequote_step

> On 02/29/2016 08:01 PM, Junio C Hamano wrote:
>> Moritz Neeb <lists@moritzneeb.de> writes:
>>
>>> Because sq_quote_argv adds a leading space (which is expected in trace.c),
>>> sq_dequote_step should remove this space again, such that the operations
>>> of quoting and dequoting are inverse of each other.
>>>
>>> This patch is preparing the way to remove some excessive trimming
>>> operation in bisect in the following commit.
>>>
>>> Signed-off-by: Moritz Neeb <lists@moritzneeb.de>
>>> ---
>>>  quote.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/quote.c b/quote.c
>>> index fe884d2..2714f27 100644
>>> --- a/quote.c
>>> +++ b/quote.c
>>> @@ -63,6 +63,8 @@ static char *sq_dequote_step(char *arg, char **next)
>>>  	char *src = arg;
>>>  	char c;
>>>  
>>> +	if (*src == ' ')
>>> +		src++;
>>>  	if (*src != '\'')
>>>  		return NULL;
>>>  	for (;;) {
>>
>> If we look at this "for (;;)" loop, we notice that (1) it accepts as
>> many spaces as there are between two quoted strings, and (2) it does
>> not limit it to SP but uses isspace().
>>
>> I wonder if you would instead want
>>
>> 	while (isspace(*src))
>>         	src++;
>>
>> to be consistent?
>>
> 
> My intention was to explicitly remove the space added by
> strbuf_addch(dst, ' ') in sq_quote_argv().
> 
> I think it would not make sense to remove more spaces, because for
> for sq_dequote() it is defined:
> 
> 	This unwraps what sq_quote() produces in place, but returns
> 	NULL if the input does not look like what sq_quote would have
> 	produced.
> 
> I understand that this counts also for the sq_dequote_array*() family.

> 
> Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>