> -----Original Message-----
> From: s...@apache.org [mailto:s...@apache.org]
> Sent: donderdag 7 mei 2015 11:48
> To: comm...@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

Reply via email to