[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Implement svnadmin verify --force

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 4 Nov 2012 02:44:58 +0200

> Index: subversion/libsvn_repos/dump.c
> ===================================================================
> --- subversion/libsvn_repos/dump.c (revision 1402414)
> +++ subversion/libsvn_repos/dump.c (working copy)
> @@ -1413,19 +1443,31 @@
> void *cancel_edit_baton;
> svn_fs_root_t *to_root;
> apr_hash_t *props;
> + svn_error_t *err;

You were asked to fix this in an earlier review.

> + if (err && keep_going)
> + {
> + notify_verification_error(rev, err, notify_func, notify_baton,
> + iterpool);
> + svn_error_clear(err);

found_corruption = TRUE;

> + return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
> + _("Repository has corruptions."));

While I'm here...

- No trailing period, per HACKING
- Why not pass NULL for the last argument?
- Document the fact that an error is returned in this case

> Index: subversion/svnadmin/main.c
> ===================================================================
> --- subversion/svnadmin/main.c (revision 1402414)
> +++ subversion/svnadmin/main.c (working copy)
> @@ -738,6 +743,16 @@
> notify->warning_str));
> return;
>
> + case svn_repos_notify_failure:
> + if (notify->revision != SVN_INVALID_REVNUM)
> + svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> + _("svnadmin: E%d: Error verifying revision %ld\n"),

-1 (layering violation). Use svn_error_quick_wrap().

While you're at it, wrap to 80 columns.
Received on 2012-11-04 01:45:37 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.