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 <[email protected]> CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
