Re: [PATCH] checksums for text bases (early review opportunity)

cmpilato@collab.net writes:
> I made the change on purpose because it was too aggressive a fix.
> That is, the error that we get back isn't always APR_ENOTDIR, yet is
> still just as harmless an error, and therefore fine enough for
> indicating that we're looking at a file instead of a dir.

Oh, that's kind of surprising. When is it not APR_ENOTDIR and yet
still means `path' is a file?

We're essentially using svn_wc__open_adm_file as a path test:

   err = svn_wc__open_adm_file (&log_fp, log_parent, SVN_WC__ADM_LOG,
                                (APR_WRITE | APR_APPEND | APR_CREATE),
                                pool);

   if (err) /* versus (err && (err->apr_err == APR_ENOTDIR)) */
     {
       /* (Ah, PATH must be a file. So create a logfile in its
          parent instead.) */
 
       [...]
     }

If we are to interpret an error as meaning `path' is a file, then the
reason the open failed better be due to an non-directory intermediate.
For example, if path is

    /home/fuzzbat/src/myproject/foo.c

then when we try to open

    /home/fuzzbat/src/myproject/foo.c/.svn/log

we should get the specific error APR_ENOTDIR, which is the errno
returned when an intermediate path component is not a directory.

[I know you're already aware of that example, it's more for others who
may just be coming to the discussion.]

If we get some error other than APR_ENOTDIR, that may mean something
other than "an intermediate dir was a file" (for example, it could be
a permission error). Therefore, I think we can't just assume that any
error at all means `path' is a file. Errors could mean all sorts of
things, and only one (or a few) of those things implies that `path' is
a file.

I could see how there might be one or two other error codes we should
check in addition to APR_ENOTDIR (though I can't guess what they would
be). But I don't see how *all* error codes can be interpreted to mean
the same thing, which is what the current code does.

?,
-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 12 20:51:19 2002