On Friday 11 November 2005 08:29, C. Michael Pilato wrote:
> Garrett Rooney <rooneg@electricjellyfish.net> writes:
> > On 11/10/05, John Szakmeister <john@szakmeister.net> wrote:
> > > I expected that if we were creating full dumps (not incremental, not
> > > delta dumps) that the md5sum would be computed and verified for each
> > > file. The same with svnadmin verify.
> > >
> > > Was this the intended behavior?
> >
> > That surprised me as well, but now that I look at dump.c it does seem
> > to be the case. It seems like we should at least be verifying the
> > checksums when we're doing a verify, if nothing else, and probably in
> > a normal dump as well.
>
> IIRC, when dump was written, BDB was the only backend, and BDB does
> MD5 checking on all reads. Does FSFS not behave similarly safety net?
Here is the relevant lines from fs_fs.c (rep_read_contents):
/* Perform checksumming. We want to check the checksum as soon as
the last byte of data is read, in case the caller never performs
a short read, but we don't want to finalize the MD5 context
twice. */
if (!rb->checksum_finalized)
{
apr_md5_update (&rb->md5_context, buf, *len);
rb->off += *len;
if (rb->off == rb->len)
{
unsigned char checksum[APR_MD5_DIGESTSIZE];
rb->checksum_finalized = TRUE;
apr_md5_final (checksum, &rb->md5_context);
if (! svn_md5_digests_match (checksum, rb->checksum))
return svn_error_createf
(SVN_ERR_FS_CORRUPT, NULL,
_("Checksum mismatch while reading representation:\n"
" expected: %s\n"
" actual: %s\n"),
svn_md5_digest_to_cstring_display (rb->checksum, rb->pool),
svn_md5_digest_to_cstring_display (checksum, rb->pool));
}
}
It appears that it does the checksumming, but it's only checked if you get to
the end of the file... which makes sense. However, if the svndiff that's
present doesn't help you to get to that point (i.e., it's shorter than it
should be), then the checksum never gets compared to the expected checksum.
I also placed some debugging info into rep_read_contents_close(), and it
appears that close isn't always called for each rep that has it's contents
read. I'm not sure if that's a bug or not. It would be nice if the routines
at least did a check to make sure that if the svndiff was used entirely, and
we still haven't reached the expanded length, then throw an error indicating
that the svndiff is incomplete or short. Does that sound fair? If so, I can
go and write a patch to implement this extra checking. I think it would be
very helpful for those of us fixing peoples' repositories (I've fixed 3 FSFS
repos in the last couple months.).
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 14 12:30:18 2005