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

Re: svn commit: r32804 - in trunk/subversion: include libsvn_wc

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Tue, 02 Sep 2008 12:52:48 -0400

julianfoad_at_tigris.org writes:
> --- subversion/include/svn_wc.h (revision 32795)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -1176,6 +1187,47 @@ typedef struct svn_wc_conflict_descripti
>
> } svn_wc_conflict_description_t;
>
> +/**
> + * Allocate an @c svn_wc_conflict_description_t structure in @a pool,
> + * initialize to represent a text conflict, and return it.
> + *
> + * Set the @c path field of the created struct to @a path, the @c access
> + * field to @a adm_access, the @c kind field to @c
> + * svn_wc_conflict_kind_text, the @c node_kind to @c svn_node_file, the @c
> + * action to @c svn_wc_conflict_action_edit, and the @c reason to @c
> + * svn_wc_conflict_reason_edited.
> + *
> + * @note: It is the caller's responsibility to set the other required fields
> + * (such as the four file names and @c mime_type and @c is_binary).
> + *
> + * @since New in 1.6.
> + */
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_text(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);

Is @a path stored by reference, or is its value copied into @a pool?
Doc string should say.

> +/**
> + * Allocate an @c svn_wc_conflict_description_t structure in @a pool,
> + * initialize to represent a property conflict, and return it.
> + *
> + * Set the @c path field of the created struct to @a path, the @c access
> + * field to @a adm_access, the @c kind field to @c
> + * svn_wc_conflict_kind_prop, the @c node_kind to @a node_kind, and the @c
> + * property_name to @a property_name.
> + *
> + * @note: It is the caller's responsibility to set the other required fields
> + * (such as the four file names and @c action and @c reason).
> + *
> + * @since New in 1.6.
> + */
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_prop(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + svn_node_kind_t node_kind,
> + const char *property_name,
> + apr_pool_t *pool);

Same here, of course, and the same question applies to the @a adm_access
and @a property_name fields.

(I can see in the implementations that it's by reference, so I guess
that makes my questioning form rhetorical only.)

This next point is unrelated to your commit, I just wanted to make a
note of it:

Below, it seems we are including the *old* commit email output too, even
though the log message and diffs are above. See the 'Modified' field
followed by old-style diffs complete with dates in their headers, etc:

> Modified:
> trunk/subversion/include/svn_wc.h
> trunk/subversion/libsvn_wc/merge.c
> trunk/subversion/libsvn_wc/props.c
> trunk/subversion/libsvn_wc/util.c
>
> Modified: trunk/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_wc.h?pathrev=32804&r1=32803&r2=32804
> ==============================================================================
> --- trunk/subversion/include/svn_wc.h Fri Aug 29 04:12:29 2008 (r32803)
> +++ trunk/subversion/include/svn_wc.h Fri Aug 29 04:13:14 2008 (r32804)
> @@ -1115,7 +1115,9 @@ typedef enum svn_wc_conflict_kind_t
> *
> * @note Fields may be added to the end of this structure in future
> * versions. Therefore, to preserve binary compatibility, users
> - * should not directly allocate structures of this type.
> + * should not directly allocate structures of this type but should use
> + * svn_wc_create_conflict_description_text() or
> + * svn_wc_create_conflict_description_prop() instead.
> *
> * @since New in 1.5.
> */
> @@ -1185,6 +1187,47 @@ typedef struct svn_wc_conflict_descripti
>
> } svn_wc_conflict_description_t;
>
> +/**
> + * Allocate an @c svn_wc_conflict_description_t structure in @a pool,
> + * initialize to represent a text conflict, and return it.
> + *
> + * Set the @c path field of the created struct to @a path, the @c access
> + * field to @a adm_access, the @c kind field to @c
> + * svn_wc_conflict_kind_text, the @c node_kind to @c svn_node_file, the @c
> + * action to @c svn_wc_conflict_action_edit, and the @c reason to @c
> + * svn_wc_conflict_reason_edited.
> + *
> + * @note: It is the caller's responsibility to set the other required fields
> + * (such as the four file names and @c mime_type and @c is_binary).
> + *
> + * @since New in 1.6.
> + */
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_text(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool);
> +
> +/**
> + * Allocate an @c svn_wc_conflict_description_t structure in @a pool,
> + * initialize to represent a property conflict, and return it.
> + *
> + * Set the @c path field of the created struct to @a path, the @c access
> + * field to @a adm_access, the @c kind field to @c
> + * svn_wc_conflict_kind_prop, the @c node_kind to @a node_kind, and the @c
> + * property_name to @a property_name.
> + *
> + * @note: It is the caller's responsibility to set the other required fields
> + * (such as the four file names and @c action and @c reason).
> + *
> + * @since New in 1.6.
> + */
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_prop(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + svn_node_kind_t node_kind,
> + const char *property_name,
> + apr_pool_t *pool);
> +
>
> /** The way in which the conflict callback chooses a course of action.
> *
>
> Modified: trunk/subversion/libsvn_wc/merge.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/merge.c?pathrev=32804&r1=32803&r2=32804
> ==============================================================================
> --- trunk/subversion/libsvn_wc/merge.c Fri Aug 29 04:12:29 2008 (r32803)
> +++ trunk/subversion/libsvn_wc/merge.c Fri Aug 29 04:13:14 2008 (r32804)
> @@ -398,24 +398,20 @@ svn_wc__merge_internal(svn_stringbuf_t *
> if (conflict_func)
> {
> svn_wc_conflict_result_t *result = NULL;
> - svn_wc_conflict_description_t cdesc;
> + svn_wc_conflict_description_t *cdesc;
>
> - cdesc.path = merge_target;
> - cdesc.node_kind = svn_node_file;
> - cdesc.kind = svn_wc_conflict_kind_text;
> - cdesc.is_binary = FALSE;
> - cdesc.mime_type = (mimeprop && mimeprop->value)
> - ? mimeprop->value->data : NULL;
> - cdesc.access = adm_access;
> - cdesc.action = svn_wc_conflict_action_edit;
> - cdesc.reason = svn_wc_conflict_reason_edited;
> - cdesc.base_file = left;
> - cdesc.their_file = right;
> - cdesc.my_file = tmp_target;
> - cdesc.merged_file = result_target;
> - cdesc.property_name = NULL;
> + cdesc = svn_wc_conflict_description_create_text(merge_target,
> + adm_access,
> + pool);
> + cdesc->is_binary = FALSE;
> + cdesc->mime_type = (mimeprop && mimeprop->value)
> + ? mimeprop->value->data : NULL,
> + cdesc->base_file = left;
> + cdesc->their_file = right;
> + cdesc->my_file = tmp_target;
> + cdesc->merged_file = result_target;
>
> - SVN_ERR(conflict_func(&result, &cdesc, conflict_baton, pool));
> + SVN_ERR(conflict_func(&result, cdesc, conflict_baton, pool));
> if (result == NULL)
> return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
> NULL, _("Conflict callback violated API:"
> @@ -714,24 +710,19 @@ svn_wc__merge_internal(svn_stringbuf_t *
> if (conflict_func)
> {
> svn_wc_conflict_result_t *result = NULL;
> - svn_wc_conflict_description_t cdesc;
> + svn_wc_conflict_description_t *cdesc;
>
> - cdesc.path = merge_target;
> - cdesc.node_kind = svn_node_file;
> - cdesc.kind = svn_wc_conflict_kind_text;
> - cdesc.is_binary = TRUE;
> - cdesc.mime_type = (mimeprop && mimeprop->value)
> - ? mimeprop->value->data : NULL;
> - cdesc.access = adm_access;
> - cdesc.action = svn_wc_conflict_action_edit;
> - cdesc.reason = svn_wc_conflict_reason_edited;
> - cdesc.base_file = left;
> - cdesc.their_file = right;
> - cdesc.my_file = tmp_target;
> - cdesc.merged_file = NULL; /* notice there is NO merged file! */
> - cdesc.property_name = NULL;
> + cdesc = svn_wc_conflict_description_create_text(merge_target,
> + adm_access, pool);
> + cdesc->is_binary = TRUE;
> + cdesc->mime_type = (mimeprop && mimeprop->value)
> + ? mimeprop->value->data : NULL,
> + cdesc->base_file = left;
> + cdesc->their_file = right;
> + cdesc->my_file = tmp_target;
> + cdesc->merged_file = NULL; /* notice there is NO merged file! */
>
> - SVN_ERR(conflict_func(&result, &cdesc, conflict_baton, pool));
> + SVN_ERR(conflict_func(&result, cdesc, conflict_baton, pool));
> if (result == NULL)
> return svn_error_create(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
> NULL, _("Conflict callback violated API:"
>
> Modified: trunk/subversion/libsvn_wc/props.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/props.c?pathrev=32804&r1=32803&r2=32804
> ==============================================================================
> --- trunk/subversion/libsvn_wc/props.c Fri Aug 29 04:12:29 2008 (r32803)
> +++ trunk/subversion/libsvn_wc/props.c Fri Aug 29 04:13:14 2008 (r32804)
> @@ -1274,7 +1274,8 @@ maybe_generate_propconflict(svn_boolean_
> return SVN_NO_ERROR;
> }
>
> - cdesc = apr_pcalloc(pool, sizeof(*cdesc));
> + cdesc = svn_wc_conflict_description_create_prop(
> + path, adm_access, is_dir ? svn_node_dir : svn_node_file, propname, pool);
>
> /* Create a tmpfile for each of the string_t's we've got. */
> if (working_val)
> @@ -1361,12 +1362,6 @@ maybe_generate_propconflict(svn_boolean_
> }
>
> /* Build the rest of the description object: */
> - cdesc->path = path;
> - cdesc->node_kind = is_dir ? svn_node_dir : svn_node_file;
> - cdesc->kind = svn_wc_conflict_kind_property;
> - cdesc->property_name = propname;
> - cdesc->access = adm_access;
> -
> if (!is_dir && working_props)
> mime_propval = apr_hash_get(working_props, SVN_PROP_MIME_TYPE,
> APR_HASH_KEY_STRING);
>
> Modified: trunk/subversion/libsvn_wc/util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/util.c?pathrev=32804&r1=32803&r2=32804
> ==============================================================================
> --- trunk/subversion/libsvn_wc/util.c Fri Aug 29 04:12:29 2008 (r32803)
> +++ trunk/subversion/libsvn_wc/util.c Fri Aug 29 04:13:14 2008 (r32804)
> @@ -291,3 +291,38 @@ svn_wc__path_switched(const char *wc_pat
>
> return SVN_NO_ERROR;
> }
> +
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_text(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> +{
> + svn_wc_conflict_description_t *conflict;
> +
> + conflict = apr_palloc(pool, sizeof(*conflict));
> + conflict->path = path;
> + conflict->node_kind = svn_node_file;
> + conflict->kind = svn_wc_conflict_kind_text;
> + conflict->access = adm_access;
> + conflict->action = svn_wc_conflict_action_edit;
> + conflict->reason = svn_wc_conflict_reason_edited;
> + return conflict;
> +}
> +
> +svn_wc_conflict_description_t *
> +svn_wc_conflict_description_create_prop(const char *path,
> + svn_wc_adm_access_t *adm_access,
> + svn_node_kind_t node_kind,
> + const char *property_name,
> + apr_pool_t *pool)
> +{
> + svn_wc_conflict_description_t *conflict;
> +
> + conflict = apr_palloc(pool, sizeof(*conflict));
> + conflict->path = path;
> + conflict->node_kind = node_kind;
> + conflict->kind = svn_wc_conflict_kind_property;
> + conflict->access = adm_access;
> + conflict->property_name = property_name;
> + return conflict;
> +}
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-02 18:53:02 CEST

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.