You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/05/07 11:48:25 UTC
svn commit: r1678151 - in /subversion/trunk/subversion/libsvn_fs_fs:
cached_data.c fs_fs.c recovery.c transaction.c
Author: stsp
Date: Thu May 7 09:48:25 2015
New Revision: 1678151
URL: http://svn.apache.org/r1678151
Log:
Follow-up to r1678150:
* subversion/libsvn_fs_fs/cached_data.c
(svn_fs_fs__get_proplist): Don't quick-wrap hash parsing errors but add an
error to the chain. This way, the hash parser's error message is preserved.
* subversion/libsvn_fs_fs/fs_fs.c
(get_node_origins_from_file): Likewise.
* subversion/libsvn_fs_fs/recovery.c
(recover_find_max_ids): Likewise.
* subversion/libsvn_fs_fs/transaction.c
(get_txn_proplist): Likewise.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
subversion/trunk/subversion/libsvn_fs_fs/recovery.c
subversion/trunk/subversion/libsvn_fs_fs/transaction.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1678151&r1=1678150&r2=1678151&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Thu May 7 09:48:25 2015
@@ -2736,7 +2736,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
svn_error_clear(svn_stream_close(stream));
- return svn_error_quick_wrapf(err,
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
_("malformed property list for node-revision '%s' in '%s'"),
id_str->data, filename);
}
@@ -2769,7 +2769,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
svn_error_clear(svn_stream_close(stream));
- return svn_error_quick_wrapf(err,
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
_("malformed property list for node-revision '%s'"),
id_str->data);
}
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1678151&r1=1678150&r2=1678151&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Thu May 7 09:48:25 2015
@@ -1917,8 +1917,9 @@ get_node_origins_from_file(svn_fs_t *fs,
*node_origins = apr_hash_make(pool);
err = svn_hash_read2(*node_origins, stream, SVN_HASH_TERMINATOR, pool);
if (err)
- return svn_error_quick_wrapf(err, _("malformed node origin data in '%s'"),
- node_origins_file);
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
+ _("malformed node origin data in '%s'"),
+ node_origins_file);
return svn_stream_close(stream);
}
Modified: subversion/trunk/subversion/libsvn_fs_fs/recovery.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/recovery.c?rev=1678151&r1=1678150&r2=1678151&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/recovery.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/recovery.c Thu May 7 09:48:25 2015
@@ -210,7 +210,7 @@ recover_find_max_ids(svn_fs_t *fs,
svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
svn_error_clear(svn_stream_close(stream));
- return svn_error_quick_wrapf(err,
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
_("malformed representation for node-revision '%s'"),
id_str->data);
}
Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1678151&r1=1678150&r2=1678151&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Thu May 7 09:48:25 2015
@@ -1146,8 +1146,8 @@ get_txn_proplist(apr_hash_t *proplist,
if (err)
{
svn_error_clear(svn_stream_close(stream));
- return svn_error_quick_wrapf(err,
- _("malformed property list in transaction '%s'"),
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
+ _("malformed transaction property list in '%s'"),
path_txn_props(fs, txn_id, pool));
}
Re: svn commit: r1678151 - in
/subversion/trunk/subversion/libsvn_fs_fs: cached_data.c fs_fs.c recovery.c
transaction.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 07, 2015 at 03:19:55PM +0200, Bert Huijben wrote:
> Your log message documents this change as 'This way, the hash parser's error message is preserved.', which is not what this change does.
The docstring of svn_error_quick_wrap fooled me. I've reverted
r1678151 and improved the docstring in r1678726. The description
of what this function does is clearer now -- at least to me it is.
Thanks.
Re: svn commit: r1678151 - in
/subversion/trunk/subversion/libsvn_fs_fs: cached_data.c fs_fs.c recovery.c
transaction.c
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 07, 2015 at 03:19:55PM +0200, Bert Huijben wrote:
> Your log message documents this change as 'This way, the hash parser's error message is preserved.', which is not what this change does.
The docstring of svn_error_quick_wrap fooled me. I've reverted
r1678151 and improved the docstring in r1678726. The description
of what this function does is clearer now -- at least to me it is.
Thanks.
RE: svn commit: r1678151 - in /subversion/trunk/subversion/libsvn_fs_fs: cached_data.c fs_fs.c recovery.c transaction.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: donderdag 7 mei 2015 11:48
> To: commits@subversion.apache.org
> Subject: svn commit: r1678151 - in /subversion/trunk/subversion/libsvn_fs_fs:
> cached_data.c fs_fs.c recovery.c transaction.c
>
> Author: stsp
> Date: Thu May 7 09:48:25 2015
> New Revision: 1678151
>
> URL: http://svn.apache.org/r1678151
> Log:
> Follow-up to r1678150:
>
> * subversion/libsvn_fs_fs/cached_data.c
> (svn_fs_fs__get_proplist): Don't quick-wrap hash parsing errors but add an
> error to the chain. This way, the hash parser's error message is preserved.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (get_node_origins_from_file): Likewise.
>
> * subversion/libsvn_fs_fs/recovery.c
> (recover_find_max_ids): Likewise.
>
> * subversion/libsvn_fs_fs/transaction.c
> (get_txn_proplist): Likewise.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> subversion/trunk/subversion/libsvn_fs_fs/recovery.c
> subversion/trunk/subversion/libsvn_fs_fs/transaction.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached
> _data.c?rev=1678151&r1=1678150&r2=1678151&view=diff
> ================================================================
> ==============
> --- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Thu May 7
> 09:48:25 2015
> @@ -2736,7 +2736,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
> svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
>
> svn_error_clear(svn_stream_close(stream));
> - return svn_error_quick_wrapf(err,
> + return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
> _("malformed property list for node-revision '%s' in '%s'"),
> id_str->data, filename);
Your log message documents this change as 'This way, the hash parser's error message is preserved.', which is not what this change does.
Before and after this change the message was preserved, but now a different error code is returned; that is a different kind of change.
Personally I would have also changed the line above this to something like:
err = svn_error_compose_creater(err, svn_stream_close(stream));
To avoid dropping more error details than necessary.
See also below for a behavior change, that also applies here.
> }
> @@ -2769,7 +2769,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
> svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
>
> svn_error_clear(svn_stream_close(stream));
> - return svn_error_quick_wrapf(err,
> + return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
> _("malformed property list for node-revision '%s'"),
> id_str->data);
Same thing applies here.
> }
The quick wrap code, does the same wrapping as the svn_error_createf(). It just explicitly doesn't alter the error code.
*and* more importantly it doesn't return an error if the inner error is not an error after all.
So this change changes the code from potentially returning NULL, to always returning an explicit error.
(Whether this is a good thing or not, requires a more closely review of te surrounding code)
Bert
RE: svn commit: r1678151 - in /subversion/trunk/subversion/libsvn_fs_fs: cached_data.c fs_fs.c recovery.c transaction.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: donderdag 7 mei 2015 11:48
> To: commits@subversion.apache.org
> Subject: svn commit: r1678151 - in /subversion/trunk/subversion/libsvn_fs_fs:
> cached_data.c fs_fs.c recovery.c transaction.c
>
> Author: stsp
> Date: Thu May 7 09:48:25 2015
> New Revision: 1678151
>
> URL: http://svn.apache.org/r1678151
> Log:
> Follow-up to r1678150:
>
> * subversion/libsvn_fs_fs/cached_data.c
> (svn_fs_fs__get_proplist): Don't quick-wrap hash parsing errors but add an
> error to the chain. This way, the hash parser's error message is preserved.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (get_node_origins_from_file): Likewise.
>
> * subversion/libsvn_fs_fs/recovery.c
> (recover_find_max_ids): Likewise.
>
> * subversion/libsvn_fs_fs/transaction.c
> (get_txn_proplist): Likewise.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> subversion/trunk/subversion/libsvn_fs_fs/recovery.c
> subversion/trunk/subversion/libsvn_fs_fs/transaction.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached
> _data.c?rev=1678151&r1=1678150&r2=1678151&view=diff
> ================================================================
> ==============
> --- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Thu May 7
> 09:48:25 2015
> @@ -2736,7 +2736,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
> svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
>
> svn_error_clear(svn_stream_close(stream));
> - return svn_error_quick_wrapf(err,
> + return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
> _("malformed property list for node-revision '%s' in '%s'"),
> id_str->data, filename);
Your log message documents this change as 'This way, the hash parser's error message is preserved.', which is not what this change does.
Before and after this change the message was preserved, but now a different error code is returned; that is a different kind of change.
Personally I would have also changed the line above this to something like:
err = svn_error_compose_creater(err, svn_stream_close(stream));
To avoid dropping more error details than necessary.
See also below for a behavior change, that also applies here.
> }
> @@ -2769,7 +2769,7 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
> svn_string_t *id_str = svn_fs_fs__id_unparse(noderev->id, pool);
>
> svn_error_clear(svn_stream_close(stream));
> - return svn_error_quick_wrapf(err,
> + return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
> _("malformed property list for node-revision '%s'"),
> id_str->data);
Same thing applies here.
> }
The quick wrap code, does the same wrapping as the svn_error_createf(). It just explicitly doesn't alter the error code.
*and* more importantly it doesn't return an error if the inner error is not an error after all.
So this change changes the code from potentially returning NULL, to always returning an explicit error.
(Whether this is a good thing or not, requires a more closely review of te surrounding code)
Bert