[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r20648 - in trunk/subversion: include libsvn_subr

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-07-14 18:54:09 CEST

On Fri, 14 Jul 2006, C. Michael Pilato wrote:
...
> > trunk/subversion/include/svn_config.h
> > trunk/subversion/libsvn_subr/config_file.c
> >
> > Log:
> > Fix broken API contract and numerous error leaks in
> > svn_config_ensure(). The broken API contract resulted an error like
> > the following when $HOME isn't accessible:
> >
> > svn: Can't check path '/home/thiru/.subversion': Permission denied
>
> How is that a broken API contract? The contact mentions exactly two cases
> in which errors should *not* be returned:
>
> - something exists but is the wrong kind
> - you fail while trying to create something
>
> The "permission denied" case you site fits neither of those. So, where's
> the problem? (I mean, besides those error leaks, which really *are* no-nos.)

Mike, thanks for the review. The API states:

 "Also don't error if trying to create something and failing -- it's
 okay for the config area or its contents not to be created."

While we're not performing a specific write operation where this error
occurs (svn_io_check_path() on CONFIG_DIR), we are returning an error
due to insufficient permissions to the file system where we plan to
subsequently create our config area and templates.

This violates the spirit of this API -- we should not return an error
due to insufficient permissions when we can bow out gracefully. Can
you suggest a better phrasing for the doc string which would make this
more explicit?

- Dan

  • application/pgp-signature attachment: stored
Received on Fri Jul 14 18:55:21 2006

This is an archived mail posted to the Subversion Dev mailing list.