> -----Original Message-----
> From: philip_at_apache.org [mailto:philip_at_apache.org]
> Sent: vrijdag 28 maart 2014 18:51
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1582845 - /subversion/trunk/subversion/libsvn_fs/fs-
> loader.c
>
> Author: philip
> Date: Fri Mar 28 17:51:09 2014
> New Revision: 1582845
>
> URL: http://svn.apache.org/r1582845
> Log:
> Fix a potential error leak.
>
> * subversion/libsvn_fs/fs-loader.c
> (svn_fs_lock, svn_fs_unlock): Handle errors in a manner similar to
> the repos functions.
>
> Found by: julianfoad
>
> Modified:
> subversion/trunk/subversion/libsvn_fs/fs-loader.c
>
> Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-
> loader.c?rev=1582845&r1=1582844&r2=1582845&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
> +++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Fri Mar 28 17:51:09
> 2014
> @@ -1677,21 +1677,30 @@ svn_fs_lock(svn_lock_t **lock, svn_fs_t
> {
> apr_hash_t *targets = apr_hash_make(pool), *results;
> svn_fs_lock_target_t target;
> - svn_fs_lock_result_t *result;
> + svn_error_t *err;
>
> target.token = token;
> target.current_rev = current_rev;
> svn_hash_sets(targets, path, &target);
>
> - SVN_ERR(svn_fs_lock2(&results, fs, targets, comment, is_dav_comment,
> - expiration_date, steal_lock, pool, pool));
> + err = svn_fs_lock2(&results, fs, targets, comment, is_dav_comment,
> + 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?
And it is certainly not our standard pattern for public functions... By default the output arguments are undefined on error.
(It looks like all the implementations handle things this way...)
Bert
> + {
> + svn_fs_lock_result_t *result
> + = svn__apr_hash_index_val(apr_hash_first(pool, results));
>
> - SVN_ERR_ASSERT(apr_hash_count(results));
> - result = svn__apr_hash_index_val(apr_hash_first(pool, results));
> - if (result->lock)
> - *lock = result->lock;
> + if (result->lock)
> + *lock = result->lock;
> +
> + if (err && result->err)
> + svn_error_compose(err, result->err);
> + else if (!err)
> + err = result->err;
> + }
>
> - return result->err;
> + return err;
> }
>
> svn_error_t *
> @@ -1717,18 +1726,26 @@ svn_fs_unlock(svn_fs_t *fs, const char *
> svn_boolean_t break_lock, apr_pool_t *pool)
> {
> apr_hash_t *targets = apr_hash_make(pool), *results;
> - svn_fs_lock_result_t *result;
> + svn_error_t *err;
>
> if (!token)
> token = "";
> svn_hash_sets(targets, path, token);
>
> - SVN_ERR(svn_fs_unlock2(&results, fs, targets, break_lock, pool, pool));
> + err = svn_fs_unlock2(&results, fs, targets, break_lock, pool, pool);
> +
> + if (apr_hash_count(results))
> + {
> + svn_fs_lock_result_t *result
> + = svn__apr_hash_index_val(apr_hash_first(pool, results));
>
> - SVN_ERR_ASSERT(apr_hash_count(results));
> - result = svn__apr_hash_index_val(apr_hash_first(pool, results));
> + if (err && result->err)
> + svn_error_compose(err, result->err);
> + else if (!err)
> + err = result->err;
> + }
>
> - return result->err;
> + return err;
> }
>
> svn_error_t *
>
Received on 2014-03-28 19:47:57 CET