From: Reece Dunn Date: Tue, 14 Dec 2010 23:36:56 GMT Subject: Re: [PATCH] git-p4: Fix 'p4 opened' in git-p4 for names with spaces Message-ID: In-Reply-To: <7vvd2wq72l.fsf@alter.siamese.dyndns.org> On 14 December 2010 23:16, Junio C Hamano wrote: > Jerzy Kozera writes: > >> Signed-off-by: Jerzy Kozera >> --- >>  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