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