On Sun, Aug 01, 2010 at 10:56:47PM +0200, Bert Huijben wrote: > Looking a bit further: Do we really need to take the absolute path for the > error message? (What do we in similar code paths?)
I would like Subversion to print helpful error message. Though the path need not be absolute, but see below. > The easiest way to fix this would be to just use the caller provided paths > in the error message. That would immediately fix the error leak. The caller-provided paths contain a trailing '/db', and running upgrade with those doesn't work: $ svnadmin upgrade repos/db subversion/svnadmin/main.c:1543: (apr_err=2) subversion/libsvn_repos/repos.c:1572: (apr_err=2) subversion/libsvn_repos/repos.c:1502: (apr_err=2) subversion/libsvn_repos/repos.c:1338: (apr_err=2) subversion/libsvn_repos/repos.c:1338: (apr_err=2) svnadmin: Error opening db lockfile subversion/libsvn_subr/io.c:1625: (apr_err=2) subversion/libsvn_subr/io.c:2714: (apr_err=2) svnadmin: Can't open file 'repos/db/locks/db.lock': No such file or directory So we cannot advise users to run the command with 'db' at the end. We could use the relative paths to avoid further SVN_ERR() calls, sure. But you've also said that you didn't want the dirname() hack to strip off the trailing 'db'. So I'm not sure how you'd like to make Subversion provide a useful error message for this case. > In this case you would need to use src_path for the error message anyway if > the svn_dirent_get_absolute() fails. That's very likely not gonna fail. We've already opened the repository. Stefan
