[ccan] New module: bdelta

Rusty Russell rusty at rustcorp.com.au
Fri Aug 19 16:17:56 EST 2011


On Tue, 16 Aug 2011 08:51:18 -0400, Joey Adams <joeyadams3.14159 at gmail.com> wrote:
> This is a new module for creating deltas between byte strings.  I
> wrote it because other diff libraries I looked at, namely libxdiff and
> librsync, appeared needlessly difficult to use.
> 
> It seems to work really well for strings that differ by a small number
> (<=1000) of bytes.  It offers no benefit for strings that differ by
> more than that, but this may be improved in a future version by adding
> a second algorithm.
> 
> It is tested and ready to apply.

I like it!  It's a perfect example of a ccan module: it does one thing,
and does it well.

A few minor nitpicks from reading the patch (I haven't applied it yet!)

1) I'm always tempted to reuse errno rather than invent my
own error codes (ENOMEM, EINVAL and EIO?), but otherwise I would do an
enum for the error codes; it just makes the API more explicit.

2) Maybe use the term 'char arrays' rather than 'byte strings' as
   string has that 0-terminated sense for C coders.

3) I wouldn't implement bdelta_perror personally, since I prefer
   err/errx/warn/warnx.

4) We really need a good bitstring library: it'd be great for this!

I think testing could be improved a bit.  For example, I'd test the
diff results directly, before testing patching.  eg. that "aaaa" vs
"aaaa" produced a non-diff, then "aaaa" vs "aaaab", etc.  Then once the
diffs pass, test those patches.

As far as I can tell, if your diff never generated anything but whole
new strings, you'd pass?

And I don't think your random test actually tests patching?

Thanks!
Rusty.


More information about the ccan mailing list