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?
[[[
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 931483)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -224,6 +224,9 @@ io_check_path(const char *path,
*kind = svn_node_none;
else if (APR_STATUS_IS_ENOTDIR(apr_err)
#ifdef WIN32
+ /* On Windows, APR_STATUS_IS_ENOTDIR includes several kinds of
+ * invalid-pathname error but not this one, so we include it. */
+ /* ### This fix should go into APR. */
|| (APR_TO_OS_ERROR(apr_err) == ERROR_INVALID_NAME)
#endif
)
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 931483)
+++ subversion/include/svn_io.h (working copy)
@@ -93,8 +93,9 @@ typedef struct svn_io_dirent_t {
* If @a path exists but is none of the above, set @a *kind to
* #svn_node_unknown.
*
- * If unable to determine @a path's kind, return an error, with @a *kind's
- * value undefined.
+ * If @a path is not a valid pathname, set @a *kind to #svn_node_none. If
+ * unable to determine @a path's kind for any other reason, return an error,
+ * with @a *kind's value undefined.
*
* Use @a pool for temporary allocations.
*
]]]
- Julian
Received on 2010-04-07 16:29:13 CEST