Re: [PATCH] Delta window changes and delta applicator

Thanks for the careful review.

> Did you look at the VCDIFF->text delta declarations in
> include/svn_delta.h? Unless I misunderstood completely, they're
> meant to read an external delta stream and produce text delta
> windows.

Er, right, I hadn't. But having read thad, we have two very different
interfaces:

        * svn_txdelta() hands you a stream, and you call
          svn_txdelta_next_window() on it until you get a null window.

        * svn_vcdiff_parse() take an svn_vcdiff_parser_t, which
          contains a window handler callback.

Having a pull-style interface for one function and a push-style
interface for the other means we can't write code like, say, a delta
applicator which works for both.

>> I also wrote some wrappers for full reads and full writes,
>> since generic read and write functions appear to be allowed
>> to return short lengths for arbitrary reasons.

> IMO it would be better to impose that restriction on the generic
> read/write functions.

That's fine with me if it's fine with other people.

>> +#ifndef MAX
>> +#define MAX(x, y) ((x) > (y) ? (x) : (y))
>> +#endif

> I must say I absolutely hate seeing min/max macros [...]

I'm not too enamored of them either. I'll look for alternatives along
the lines of what you say.

> (BTW, you put "int new_size" in the bit of code where you allocate
> the target buffer; should be apr_size_t. Also, it looks as if
> tview_len and sview_len in the window should be apr_size_t, too.)

Will change.
Received on Sat Oct 21 14:36:08 2006