rsync - Jun 2002 - batch diffs

I needed a tool to produce diffs of binary volumes, and to apply them as
patches accordingly. So I tried the 'batch mode' that was added to
rsync,
but it invariably segfaulted on reading the batches...

	$ sh foo.rsync_argvs
	foo.rsync_argvs: line 1:  8971 Segmentation fault      rsync -a -v --stats
--read-batch=foo ${1:-FOO}
	rsync: error writing 64 unbuffered bytes - exiting: Broken pipe
	rsync error: error in rsync protocol data stream (code 12) at io.c(463)

	$ rsync --version
	rsync  version 2.5.5  protocol version 26
...
	Capabilities: 64-bit files, socketpairs, hard links, symlinks, batchfiles, 
	              no IPv6, 64-bit system inums, 64-bit internal inums

  Digging around in the sources (uhg), I found that this occurs because...
batch.c line 485 dereferences s->sums[i] which is NULL, because... generator
thinks it can disable_deltas_p() when batches are read.

  I'm not sure about the usefulness of this batch support, but the ability
to create rsync diffs would be awesome. (Without updating the destination,
and why all that checksum data?) Would it not be better to provide separate
utiliti(es) for this purpose and not clutter up rsync code?

-------------- next part --------------
diff -ru rsync-2.5.5.orig/generator.c rsync-2.5.5/generator.c
--- rsync-2.5.5.orig/generator.c	Mon Mar 25 07:54:31 2002
+++ rsync-2.5.5/generator.c	Sun Jun  2 20:25:54 2002
@@ -141,6 +141,7 @@
 	extern int whole_file, no_whole_file;
 	extern int local_server;
 	extern int write_batch;
+	extern int read_batch;
 
 	assert(whole_file == 0 || whole_file == 1);
 
@@ -150,7 +151,7 @@
 		return True;
 	else if (no_whole_file)
 		return False;
-	else if (write_batch)
+	else if (write_batch || read_batch)
 		return False;
 	else
 		return local_server;

On Sun, Jun 02, 2002 at 10:40:34PM +0300, Kalev Soikonen
wrote:
>   Digging around in the sources (uhg), I found that this occurs because...
> batch.c line 485 dereferences s->sums[i] which is NULL, because...
generator
> thinks it can disable_deltas_p() when batches are read.
Ouch. This sounds remarkably similar to something Dave Dykstra wrote on
Wed, 23 Jan 2002:
<quote>
So, I hadn't gotten a chance to try your patch, but Martin checked in your
changes into CVS and I pulled them back out of there and tried them out.        
It is definitely working better.  I'm still seeing a rsync_argvs file
getting created in my home directory on the remote machine; --read-batch        
is also printing out "batch file extension" with two different numbers
so
that was my clue.  One time I accidentally tried use --read-batch to            
directory that didn't match the destination directory that was present
when --write-batch created the files, and then it core dumped at batch.c        
line 487:                                                                       
                        if ((s->sums[i].sum1 != file_sum1) ||
                            (memcmp                                             
                             (s->sums[i].sum2, file_sum2,
                              csum_length) != 0)) {                             
                                *checksums_match = 0;                           
and gdb said that s->sums[i] was a bad address.  s->sums[0] was OK, but i
was 487.  So, more care needs to be taken with making sure that the             
new destination matches the original destination.
</quote>

Because disable_deltas_p() currently returns true in recv_generator():446,
s->count == 0 but since read_batch_csum_info() does not check if s->count
> 0
before trying to derefence s->sums[]i, rsync crashes. So for additional
robustness read_batch_csum_info() should check s->count. Does that sound
reasonable?
>   I'm not sure about the usefulness of this batch support, but the
ability
> to create rsync diffs would be awesome. (Without updating the destination,
> and why all that checksum data?) Would it not be better to provide separate
> utiliti(es) for this purpose and not clutter up rsync code?
Ideally, yes. Currently, rsync's batch mode is the only method of creating
and
using diffs this way that I am aware of.

-- 
Jos Backus                 _/  _/_/_/        Santa Clara, CA
                          _/  _/   _/
                         _/  _/_/_/             
                    _/  _/  _/    _/
jos@catnook.com     _/_/   _/_/_/            use Std::Disclaimer;