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

Re: Status of the branch diff-optimizations-bytes branch

From: Johan Corveleyn <jcorvel_at_gmail.com>
Date: Thu, 27 Jan 2011 22:08:42 +0100

On Thu, Jan 27, 2011 at 4:52 PM, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> Johan,
> I'd appreciate review on the attached patch.  It is an attempt to rev
> the svn_diff_fns_t struct and related functions.  You'll notice that I
> commented out the use of datasources_open in both diff_file.c and
> diff_memory.c, in an attempt to exercise the deprecated function
> wrappers.
>
> It currently segfaults horribly, but I'm not completely sure why.  I
> suspect something is amiss in the implementation of the compat wrapper
> for datasources_open in deprecated.c.  If you'd review this patch, I'd
> be much obliged.

I don't completely grok the rev'ing and wrapping stuff just yet, but
giving it a try ...

One thing looked suspicious to me:

> Index: subversion/libsvn_diff/deprecated.c
> ===================================================================
> --- subversion/libsvn_diff/deprecated.c (revision 1064135)
> +++ subversion/libsvn_diff/deprecated.c (working copy)
> @@ -41,7 +41,61 @@
>
>
> /*** Code. ***/
> +struct fns_wrapper_baton
> +{
> + /* We put the old baton in front of this one, so that we can still use
> + this baton in place of the old. This prevents us from having to
> + implement simple wrappers around each member of diff_fns_t. */
> + void *old_baton;
> + const svn_diff_fns_t *vtable;
> +};
>
> +static svn_error_t *
> +datasources_open(void *baton, apr_off_t *prefix_lines,
> + svn_diff_datasource_e datasources[],
> + apr_size_t datasource_len)
> +{
> + struct fns_wrapper_baton *fwb = baton;
> + int i;
> +
> + /* Just iterate over the datasources, using the old singular version. */
> + for (i = 0; i < datasource_len; i++)
> + {
> + SVN_ERR(fwb->vtable->datasource_open(baton, datasources[i]));

Not sure, but shouldn't that be 'baton->old_baton' instead of 'baton'?

> + }
> +
> + /* Don't claim any prefix matches. */
> + *prefix_lines = 0;
> +
> + return SVN_NO_ERROR;
> +}

The rest looks ok, at least as far as I understand the rev'ing stuff ...

Cheers,

-- 
Johan
Received on 2011-01-27 22:09:42 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.