batch diffs

Jos Backus jos at catnook.com
Sun Jun 2 14:48:02 EST 2002


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 at catnook.com     _/_/   _/_/_/            use Std::Disclaimer;




More information about the rsync mailing list