> -----Original Message----- > From: 'Stefan Sperling' [mailto:s...@elego.de] > Sent: zondag 1 augustus 2010 23:09 > To: Bert Huijben > Cc: dev@subversion.apache.org > Subject: Re: svn commit: r980811 - in > /subversion/trunk/subversion/libsvn_fs_fs: fs.h fs_fs.c > > 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.
If svn_dirent_get_absolute() returns an error, the output argument of that function is undefined. And you can't use an undefined pointer as a string in an error message. (Just this afternoon I was fixing a bug where the error message should have contained a url, but had the content of fsfs.conf...) Bert > > Stefan