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

Re: svn commit: rev 1752 - trunk/subversion trunk/subversion/include trunk/subversion/libsvn_diff

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-04-25 01:43:23 CEST

striker@tigris.org writes:

> +
> +} svn_diff_datasource_e;
> +
> +
> +typedef struct svn_diff_fns_t
> +{
> + apr_status_t (*datasource_open)(void *diff_baton,
> + svn_diff_datasource_e datasoure);

If this is a subversion library why is it returning apr_status_t and
not svn_error_t*? This question applies to most, if not all, the
error return types.

> +
> + /* ### Unused at the moment.
> + */
> + apr_status_t (*datasource_seek)(void *diff_baton,
> + svn_diff_datasource_e datasoure,
> + apr_off_t token_offset);
> +
> + void *(*datasource_get_token)(void *diff_baton,
> + svn_diff_datasource_e datasoure);

I am suspicious of this function, it does not appear able to return an
error. What is the point of abstracting the datasource, is it to allow
an 'intelligent' datasource to handle large files in chunks? If so
these functions will almost certainly need to be able to return
errors.

> +
> + void (*datasource_close)(void *diff_baton,
> + svn_diff_datasource_e datasoure);
> +
> + int (*token_compare)(void *diff_baton,
> + void *ltoken,
> + void *rtoken);
> +
> + void (*token_discard)(void *diff_baton,
> + void *token);
> +
> + void (*token_discard_all)(void *diff_baton);
> +} svn_diff_fns_t;
> +
> +/* Produce a diff between baseline and workingcopy.
> + *
> + */
> +apr_status_t svn_diff(svn_diff_t **diff,
> + void *diff_baton,
> + svn_diff_fns_t *diff_fns,
> + apr_pool_t *pool);
> +
> +/* Merge baseline, workingcopy and repository.
> + *
> + */
> +apr_status_t svn_diff3(svn_diff_t **diff,
> + void *diff_baton,
> + svn_diff_fns_t *diff_fns,
> + apr_pool_t *pool);
> +
> +
> +/*
> + *
> + */
> +typedef struct svn_diff_output_fns_t
> +{
> + void (*output_common)(void *output_baton,
> + apr_off_t baseline_start,
> + apr_off_t baseline_end,
> + apr_off_t workingcopy_start,
> + apr_off_t workingcopy_end,
> + apr_off_t repository_start,
> + apr_off_t repository_end);

Karl already mentioned the assumption about baseline, working
copy. Consider 'svn diff -rREV1:REV2' neither of the files being
compared is part of a baseline or a working copy.

> + void (*output_conflict)(void *output_baton,
> + apr_off_t baseline_start,
> + apr_off_t baseline_end,
> + apr_off_t workingcopy_start,
> + apr_off_t workingcopy_end,
> + apr_off_t repository_start,
> + apr_off_t repository_end);
> + void (*output_diff_workingcopy)(void *output_baton,
> + apr_off_t baseline_start,
> + apr_off_t baseline_end,
> + apr_off_t workingcopy_start,
> + apr_off_t workingcopy_end,
> + apr_off_t repository_start,
> + apr_off_t repository_end);
> + void (*output_diff_repository)(void *output_baton,
> + apr_off_t baseline_start,
> + apr_off_t baseline_end,
> + apr_off_t workingcopy_start,
> + apr_off_t workingcopy_end,
> + apr_off_t repository_start,
> + apr_off_t repository_end);
> + void (*output_diff_common)(void *output_baton,
> + apr_off_t baseline_start,
> + apr_off_t baseline_end,
> + apr_off_t workingcopy_start,
> + apr_off_t workingcopy_end,
> + apr_off_t repository_start,
> + apr_off_t repository_end);
> +} svn_diff_output_fns_t;
> +

Why don't these functions return an error status? I am assuming that
these functions are responsible for writing the merged file, or the
context diff (that's a guess there's not much documentation). If the
disk fills up while merging an update I want subversion to complain
loudly, I don't want it to carry on!

-- 
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 25 01:44:30 2002

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