Re: [PATCH] git-p4: Fix 'p4 opened' in git-p4 for names with spaces

[Reece Dunn <msclrhd@googlemail.com>]

On 14 December 2010 23:16, Junio C Hamano <gitster@pobox.com> wrote:
> Jerzy Kozera <jerzy.kozera@gmail.com> writes:
>
>> Signed-off-by: Jerzy Kozera <jerzy.kozera@gmail.com>
>> ---
>>  contrib/fast-import/git-p4 |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>> index 04ce7e3..a5297e7 100755
>> --- a/contrib/fast-import/git-p4
>> +++ b/contrib/fast-import/git-p4
>> @@ -144,7 +144,7 @@ def setP4ExecBit(file, mode):
>>  def getP4OpenedType(file):
>>      # Returns the perforce file type for the given file.
>>
>> -    result = p4_read_pipe("opened %s" % file)
>> +    result = p4_read_pipe("opened \"%s\"" % file)
>
> Don't you need a lot more than that?  What if file has " or \ in it?

Those are invalid characters for a filename on Windows, so cannot be
entered/present in the filename. On Linux, they are accepted, but
don't get put into the filename, so it all depends on where the data
for file comes from (API call or user/external source). Not sure how
Mac/BSD/Solaris handle those characters.

This looks fine to me, but I wonder if there are other places
referencing file paths that require quoting to correctly handle
spaces.

Also, escaping the quote characters can be avoided by using single
quoted string literals:

+    result = p4_read_pipe('opened "%s"' % file)

- Reece