> -----Original Message-----
> From: Philip Martin [mailto:philip.martin_at_wandisco.com]
> Sent: vrijdag 28 maart 2014 20:57
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org; commits_at_subversion.apache.org
> Subject: Re: svn commit: r1582845 -
> /subversion/trunk/subversion/libsvn_fs/fs-loader.c
>
> "Bert Huijben" <bert_at_qqmail.nl> writes:
>
> >> + expiration_date, steal_lock, pool, pool);
> >> +
> >> + if (apr_hash_count(results))
> >
> > Is this function explicitly documented to always set the results value
> > on all error paths?
> >
> > I don't see that in the documentation for svn_fs_lock2?
>
> It is now.
>
> > And it is certainly not our standard pattern for public
> > functions... By default the output arguments are undefined on error.
>
> It's necessary to allow the caller to handle all the errors.
Perhaps it would be easier for all callers if the (inner) handler that
allocates the hash table installs a cleanup handler on it that releases all
the svn_error_t instances on pool cleanup of the release pool?
Now all callers that have access to the results have to cover for the
hopefully unlikely case where there are both global and per node errors,
while such a cleanup handler would allow all of them to use the standard
error pattern.
(The code iterating the hash would also have to cover for errors further in
the hash table, etc.)
Bert
>
> --
> Philip Martin | Subversion Committer
> WANdisco // *Non-Stop Data*
Received on 2014-03-29 10:36:49 CET