[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: r877014 - on Windows, invalid path => svn_node_none [was: svn commit: r930333 - /subversion/branches/1.6.x/STATUS]

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Wed, 07 Apr 2010 10:32:32 -0400

Julian Foad wrote:
> C. Michael Pilato wrote:
>> Bert Huijben wrote:
>>> Before r877014 we returned an error on paths as
>>> "C:\path\that\is:invalid", but we didn't return an error on an equally
>>> invalid path "//q:" (format is "//server/share" or "q:/") that happens to
>>> return ERROR_BAD_PATHNAME instead of ERROR_INVALID_NAME because it is
>>> handled in another layer of the Windows path redirector.
>>>
>>> So what I tried to say is that the APR_STATUS_IS_ENOENT() and
>>> APR_STATUS_IS_ENOTDIR() checks catch almost every path syntax error, but
>>> not this specific one. I fixed this by also handling this error like the
>>> other bad pathnames. An equally valid (or possibly better) route is to
>>> handle all these invalid path errors as an error.
>> FWIW, I found your prior explanation of why you prefer this route of fix
>> both informative and compelling. What lacks is in-code documentation of why
>> devs would see what appears to the casual passer-by as just some
>> out-of-place special-casing. (That's a lot of hyphens.) Surely others will
>> wonder why ENOENT and ENOTDIR aren't sufficient in this case.
>
> +1.
>
> Is this patch good?

-1. I prefer my comments in green.

(Just kidding. +1)

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on 2010-04-07 16:33:05 CEST

This is an archived mail posted to the Subversion Dev mailing list.