You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2011/02/04 16:20:52 UTC
svn commit: r1067195 - in /subversion/trunk/subversion:
include/svn_checksum.h libsvn_client/export.c libsvn_subr/checksum.c
Author: hwright
Date: Fri Feb 4 15:20:50 2011
New Revision: 1067195
URL: http://svn.apache.org/viewvc?rev=1067195&view=rev
Log:
Allow callers of svn_checksum_mismatch_err() to specify the first bit of their
error message. Also, remove a call to apr_psprintf when generating the error
message.
* subversion/libsvn_subr/checksum.c
(svn_checksum_mismatch_err): Use varargs to generate the error message.
* subversion/include/svn_checksum.h
(svn_checksum_mismatch_err): Update parameters to accept a varargs message.
* subversion/libsvn_client/export.c
(close_file): Update caller.
Modified:
subversion/trunk/subversion/include/svn_checksum.h
subversion/trunk/subversion/libsvn_client/export.c
subversion/trunk/subversion/libsvn_subr/checksum.c
Modified: subversion/trunk/subversion/include/svn_checksum.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_checksum.h?rev=1067195&r1=1067194&r2=1067195&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_checksum.h (original)
+++ subversion/trunk/subversion/include/svn_checksum.h Fri Feb 4 15:20:50 2011
@@ -241,9 +241,9 @@ svn_checksum_size(const svn_checksum_t *
/**
- * Return an error of type #SVN_ERR_CHECKSUM_MISMATCH for @a object_label,
- * using the @a actual and @a expected checksums to create the error
- * message, if the two checksums don't match. Otherwise, return #SVN_NO_ERROR.
+ * Return an error of type #SVN_ERR_CHECKSUM_MISMATCH if @a actual and
+ * @a expected checksums do not match, otherwise, return #SVN_NO_ERROR.
+ * Use @a fmt, and the following parameters to populate the error message.
*
* @a scratch_pool is used for temporary allocations; the returned error
* will be allocated in its own pool (as is typical).
@@ -251,10 +251,12 @@ svn_checksum_size(const svn_checksum_t *
* @since New in 1.7.
*/
svn_error_t *
-svn_checksum_mismatch_err(const char *object_label,
- const svn_checksum_t *expected,
+svn_checksum_mismatch_err(const svn_checksum_t *expected,
const svn_checksum_t *actual,
- apr_pool_t *scratch_pool);
+ apr_pool_t *scratch_pool,
+ const char *fmt,
+ ...)
+ __attribute__ ((format(printf, 4, 5)));
/**
Modified: subversion/trunk/subversion/libsvn_client/export.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/export.c?rev=1067195&r1=1067194&r2=1067195&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/export.c (original)
+++ subversion/trunk/subversion/libsvn_client/export.c Fri Feb 4 15:20:50 2011
@@ -900,8 +900,9 @@ close_file(void *file_baton,
actual_checksum = svn_checksum__from_digest(fb->text_digest,
svn_checksum_md5, pool);
- SVN_ERR(svn_checksum_mismatch_err(svn_dirent_local_style(fb->path, pool),
- text_checksum, actual_checksum, pool));
+ SVN_ERR(svn_checksum_mismatch_err(text_checksum, actual_checksum, pool,
+ _("Checksum mismatch for '%s'"),
+ svn_dirent_local_style(fb->path, pool)));
if ((! fb->eol_style_val) && (! fb->keywords_val) && (! fb->special))
{
Modified: subversion/trunk/subversion/libsvn_subr/checksum.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/checksum.c?rev=1067195&r1=1067194&r2=1067195&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/checksum.c (original)
+++ subversion/trunk/subversion/libsvn_subr/checksum.c Fri Feb 4 15:20:50 2011
@@ -402,21 +402,28 @@ svn_checksum_size(const svn_checksum_t *
}
svn_error_t *
-svn_checksum_mismatch_err(const char *object_label,
- const svn_checksum_t *expected,
+svn_checksum_mismatch_err(const svn_checksum_t *expected,
const svn_checksum_t *actual,
- apr_pool_t *scratch_pool)
+ apr_pool_t *scratch_pool,
+ const char *fmt,
+ ...)
{
if (!svn_checksum_match(expected, actual))
{
+ va_list ap;
+ const char *desc;
+
+ va_start(ap, fmt);
+ desc = apr_pvsprintf(scratch_pool, fmt, ap);
+ va_end(ap);
+
return svn_error_createf(SVN_ERR_CHECKSUM_MISMATCH, NULL,
- apr_psprintf(scratch_pool, "%s:\n%s\n%s\n",
- _("Checksum mismatch for '%s'\n"),
- _(" expected: %s"),
- _(" actual: %s")),
- object_label,
- svn_checksum_to_cstring_display(expected, scratch_pool),
- svn_checksum_to_cstring_display(actual, scratch_pool));
+ _("%s:\n"
+ " expected: %s\n"
+ " actual: %s\n"),
+ desc,
+ svn_checksum_to_cstring_display(expected, scratch_pool),
+ svn_checksum_to_cstring_display(actual, scratch_pool));
}
else
return SVN_NO_ERROR;
Re: svn commit: r1067195 - in /subversion/trunk/subversion:
include/svn_checksum.h libsvn_client/export.c libsvn_subr/checksum.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Fri, Feb 04, 2011 at 09:28:04 -0600:
> Reviewing my own commits...
>
> On Fri, Feb 4, 2011 at 9:20 AM, <hw...@apache.org> wrote:
> > +++ subversion/trunk/subversion/libsvn_client/export.c Fri Feb 4 15:20:50 2011
> > @@ -900,8 +900,9 @@ close_file(void *file_baton,
> > actual_checksum = svn_checksum__from_digest(fb->text_digest,
> > svn_checksum_md5, pool);
> >
> > - SVN_ERR(svn_checksum_mismatch_err(svn_dirent_local_style(fb->path, pool),
> > - text_checksum, actual_checksum, pool));
> > + SVN_ERR(svn_checksum_mismatch_err(text_checksum, actual_checksum, pool,
> > + _("Checksum mismatch for '%s'"),
> > + svn_dirent_local_style(fb->path, pool)));
>
> Although the use of svn_checksum_mismatch_err() simplifies the code
> and consolidates various messages for translation, it does have one
> drawback: the call to svn_dirent_local_style() is now unconditional,
> rather than only being invoked in the (unlikely) event of a checksum
> mismatch. Is this enough of a drawback that I should revert the past
> couple of commits in this area?
>
Does it compute that string now for every path in an exported tree?
+1 to avoiding that if possible.
> -Hyrum
>
> >
> > if ((! fb->eol_style_val) && (! fb->keywords_val) && (! fb->special))
> > {
> >
> > Modified: subversion/trunk/subversion/libsvn_subr/checksum.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/checksum.c?rev=1067195&r1=1067194&r2=1067195&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/libsvn_subr/checksum.c (original)
> > +++ subversion/trunk/subversion/libsvn_subr/checksum.c Fri Feb 4 15:20:50 2011
> > @@ -402,21 +402,28 @@ svn_checksum_size(const svn_checksum_t *
> > }
> >
> > svn_error_t *
> > -svn_checksum_mismatch_err(const char *object_label,
> > - const svn_checksum_t *expected,
> > +svn_checksum_mismatch_err(const svn_checksum_t *expected,
> > const svn_checksum_t *actual,
> > - apr_pool_t *scratch_pool)
> > + apr_pool_t *scratch_pool,
> > + const char *fmt,
> > + ...)
> > {
> > if (!svn_checksum_match(expected, actual))
> > {
> > + va_list ap;
> > + const char *desc;
> > +
> > + va_start(ap, fmt);
> > + desc = apr_pvsprintf(scratch_pool, fmt, ap);
> > + va_end(ap);
> > +
> > return svn_error_createf(SVN_ERR_CHECKSUM_MISMATCH, NULL,
> > - apr_psprintf(scratch_pool, "%s:\n%s\n%s\n",
> > - _("Checksum mismatch for '%s'\n"),
> > - _(" expected: %s"),
> > - _(" actual: %s")),
> > - object_label,
> > - svn_checksum_to_cstring_display(expected, scratch_pool),
> > - svn_checksum_to_cstring_display(actual, scratch_pool));
> > + _("%s:\n"
> > + " expected: %s\n"
> > + " actual: %s\n"),
> > + desc,
> > + svn_checksum_to_cstring_display(expected, scratch_pool),
> > + svn_checksum_to_cstring_display(actual, scratch_pool));
> > }
> > else
> > return SVN_NO_ERROR;
> >
> >
> >
Re: svn commit: r1067195 - in /subversion/trunk/subversion:
include/svn_checksum.h libsvn_client/export.c libsvn_subr/checksum.c
Posted by Julian Foad <ju...@wandisco.com>.
Hyrum K Wright wrote:
> > - SVN_ERR(svn_checksum_mismatch_err(svn_dirent_local_style(fb->path, pool),
> > - text_checksum, actual_checksum, pool));
> > + SVN_ERR(svn_checksum_mismatch_err(text_checksum, actual_checksum, pool,
> > + _("Checksum mismatch for '%s'"),
> > + svn_dirent_local_style(fb->path, pool)));
>
> Although the use of svn_checksum_mismatch_err() simplifies the code
> and consolidates various messages for translation, it does have one
> drawback: the call to svn_dirent_local_style() is now unconditional,
> rather than only being invoked in the (unlikely) event of a checksum
> mismatch. Is this enough of a drawback that I should revert the past
> couple of commits in this area?
Maybe this indicates that the new public API should be one that simply
generates a checksum-mismatch error message (but doesn't do the
comparison), and a private/local wrapper should be used when it is
desired to combine the comparison and the error message creation into a
single call.
- Julian
Re: svn commit: r1067195 - in /subversion/trunk/subversion:
include/svn_checksum.h libsvn_client/export.c libsvn_subr/checksum.c
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
Reviewing my own commits...
On Fri, Feb 4, 2011 at 9:20 AM, <hw...@apache.org> wrote:
> Author: hwright
> Date: Fri Feb 4 15:20:50 2011
> New Revision: 1067195
>
> URL: http://svn.apache.org/viewvc?rev=1067195&view=rev
> Log:
> Allow callers of svn_checksum_mismatch_err() to specify the first bit of their
> error message. Also, remove a call to apr_psprintf when generating the error
> message.
>
> * subversion/libsvn_subr/checksum.c
> (svn_checksum_mismatch_err): Use varargs to generate the error message.
>
> * subversion/include/svn_checksum.h
> (svn_checksum_mismatch_err): Update parameters to accept a varargs message.
>
> * subversion/libsvn_client/export.c
> (close_file): Update caller.
>
> Modified:
> subversion/trunk/subversion/include/svn_checksum.h
> subversion/trunk/subversion/libsvn_client/export.c
> subversion/trunk/subversion/libsvn_subr/checksum.c
>
> Modified: subversion/trunk/subversion/include/svn_checksum.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_checksum.h?rev=1067195&r1=1067194&r2=1067195&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_checksum.h (original)
> +++ subversion/trunk/subversion/include/svn_checksum.h Fri Feb 4 15:20:50 2011
> @@ -241,9 +241,9 @@ svn_checksum_size(const svn_checksum_t *
>
>
> /**
> - * Return an error of type #SVN_ERR_CHECKSUM_MISMATCH for @a object_label,
> - * using the @a actual and @a expected checksums to create the error
> - * message, if the two checksums don't match. Otherwise, return #SVN_NO_ERROR.
> + * Return an error of type #SVN_ERR_CHECKSUM_MISMATCH if @a actual and
> + * @a expected checksums do not match, otherwise, return #SVN_NO_ERROR.
> + * Use @a fmt, and the following parameters to populate the error message.
> *
> * @a scratch_pool is used for temporary allocations; the returned error
> * will be allocated in its own pool (as is typical).
> @@ -251,10 +251,12 @@ svn_checksum_size(const svn_checksum_t *
> * @since New in 1.7.
> */
> svn_error_t *
> -svn_checksum_mismatch_err(const char *object_label,
> - const svn_checksum_t *expected,
> +svn_checksum_mismatch_err(const svn_checksum_t *expected,
> const svn_checksum_t *actual,
> - apr_pool_t *scratch_pool);
> + apr_pool_t *scratch_pool,
> + const char *fmt,
> + ...)
> + __attribute__ ((format(printf, 4, 5)));
>
>
> /**
>
> Modified: subversion/trunk/subversion/libsvn_client/export.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/export.c?rev=1067195&r1=1067194&r2=1067195&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/export.c (original)
> +++ subversion/trunk/subversion/libsvn_client/export.c Fri Feb 4 15:20:50 2011
> @@ -900,8 +900,9 @@ close_file(void *file_baton,
> actual_checksum = svn_checksum__from_digest(fb->text_digest,
> svn_checksum_md5, pool);
>
> - SVN_ERR(svn_checksum_mismatch_err(svn_dirent_local_style(fb->path, pool),
> - text_checksum, actual_checksum, pool));
> + SVN_ERR(svn_checksum_mismatch_err(text_checksum, actual_checksum, pool,
> + _("Checksum mismatch for '%s'"),
> + svn_dirent_local_style(fb->path, pool)));
Although the use of svn_checksum_mismatch_err() simplifies the code
and consolidates various messages for translation, it does have one
drawback: the call to svn_dirent_local_style() is now unconditional,
rather than only being invoked in the (unlikely) event of a checksum
mismatch. Is this enough of a drawback that I should revert the past
couple of commits in this area?
-Hyrum
>
> if ((! fb->eol_style_val) && (! fb->keywords_val) && (! fb->special))
> {
>
> Modified: subversion/trunk/subversion/libsvn_subr/checksum.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/checksum.c?rev=1067195&r1=1067194&r2=1067195&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/checksum.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/checksum.c Fri Feb 4 15:20:50 2011
> @@ -402,21 +402,28 @@ svn_checksum_size(const svn_checksum_t *
> }
>
> svn_error_t *
> -svn_checksum_mismatch_err(const char *object_label,
> - const svn_checksum_t *expected,
> +svn_checksum_mismatch_err(const svn_checksum_t *expected,
> const svn_checksum_t *actual,
> - apr_pool_t *scratch_pool)
> + apr_pool_t *scratch_pool,
> + const char *fmt,
> + ...)
> {
> if (!svn_checksum_match(expected, actual))
> {
> + va_list ap;
> + const char *desc;
> +
> + va_start(ap, fmt);
> + desc = apr_pvsprintf(scratch_pool, fmt, ap);
> + va_end(ap);
> +
> return svn_error_createf(SVN_ERR_CHECKSUM_MISMATCH, NULL,
> - apr_psprintf(scratch_pool, "%s:\n%s\n%s\n",
> - _("Checksum mismatch for '%s'\n"),
> - _(" expected: %s"),
> - _(" actual: %s")),
> - object_label,
> - svn_checksum_to_cstring_display(expected, scratch_pool),
> - svn_checksum_to_cstring_display(actual, scratch_pool));
> + _("%s:\n"
> + " expected: %s\n"
> + " actual: %s\n"),
> + desc,
> + svn_checksum_to_cstring_display(expected, scratch_pool),
> + svn_checksum_to_cstring_display(actual, scratch_pool));
> }
> else
> return SVN_NO_ERROR;
>
>
>