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

Re: Mismatched backwards compat callbacks in libsvn_wc

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 10 Jul 2009 21:36:15 +0100

On Fri, Jul 03, 2009 at 01:06:23AM +0400, Роман Донченко wrote:
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -file_changed(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_file_changed(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *contentstate,
> svn_wc_notify_state_t *propstate,
> svn_boolean_t *tree_conflicted,

Please indent the other parameters, too, so that they all align again
as they used to.

> @@ -585,7 +585,7 @@
> apr_hash_t *originalprops,
> void *diff_baton)
> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -605,7 +605,7 @@
>
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -file_added(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_file_added(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *contentstate,
> svn_wc_notify_state_t *propstate,
> svn_boolean_t *tree_conflicted,

Same here.

> @@ -620,7 +620,7 @@
> apr_hash_t *originalprops,
> void *diff_baton)
> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -638,7 +638,7 @@
>
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -file_deleted(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_file_deleted(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *state,
> svn_boolean_t *tree_conflicted,
> const char *path,

Same here.

> @@ -649,7 +649,7 @@
> apr_hash_t *originalprops,
> void *diff_baton)
> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -663,14 +663,14 @@
>
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -dir_added(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_dir_added(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *state,
> svn_boolean_t *tree_conflicted,
> const char *path,
> svn_revnum_t rev,
> void *diff_baton)

Same here.

> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -680,13 +680,13 @@
>
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -dir_deleted(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_dir_deleted(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *state,
> svn_boolean_t *tree_conflicted,
> const char *path,
> void *diff_baton)

Somehow it feels like I've seen this before...

> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -696,7 +696,7 @@
>
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t. */
> static svn_error_t *
> -dir_props_changed(svn_wc_adm_access_t *adm_access,
> +wrap_3to1_dir_props_changed(svn_wc_adm_access_t *adm_access,
> svn_wc_notify_state_t *state,
> svn_boolean_t *tree_conflicted,
> const char *path,

When I play Mario Kart I always get blue shells!

> @@ -704,7 +704,7 @@
> apr_hash_t *originalprops,
> void *diff_baton)
> {
> - struct callbacks_wrapper_baton *b = diff_baton;
> + struct diff_callbacks_wrapper_baton *b = diff_baton;
>
> if (tree_conflicted)
> *tree_conflicted = FALSE;
> @@ -716,7 +716,7 @@
> /* An svn_wc_diff_callbacks3_t function for wrapping svn_wc_diff_callbacks_t
> and svn_wc_diff_callbacks2_t. */
> static svn_error_t *
> -dir_opened(svn_wc_adm_access_t *adm_access,
> +wrap_3to1or2_dir_opened(svn_wc_adm_access_t *adm_access,
> svn_boolean_t *tree_conflicted,
> const char *path,
> svn_revnum_t rev,

Well it keeps going on like this and I can't think of more
inventive things to say.

I've scanned the rest but couldn't find anything wrong with it.
The compiler here is also happy with the patch applied.
With fixed parameter indentation, I'd say +1.

svn patch is not able to digest this diff by the way.
  svn: In file 'subversion/libsvn_client/patch.c' line 2201: assertion
  failed (target->current_line == hunk_start)
  Aborted (core dumped)
But that's likely to be entirely my own fault and not yours, so it does
not count towards the review :)

Thanks,
Stefan
Received on 2009-07-10 22:36:46 CEST

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