You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2005/11/07 20:52:05 UTC

Re: wc-replacements: text base checksums

On 9/26/05, Ivan Zhakov <ch...@gmail.com> wrote:
> Hi!
> I studying working copy checksums for text base and revert base
> behavior. I wonder that checksums is optional for text base.

Since we have upgraded the working copy format number, we can now
require checksums. You'll need to check whether the format is
<whatever-our-current-version-is> or higher. Below this version, we'll
still need to allow for non-existing md5's for backward compatibility.

When the wc-format is upgraded, md5's will have to be calculated for
all files which miss them.

> For
> example if we somewhere forget write checksum for text base, working
> copy would silently apply and commit txt-delta without checking
> checksum:
> subversion/libsvn_wc/update_editor.c:apply_textdelta()
> >  /* Only compare checksums this file has an entry, and the entry has
> >     a checksum.  If there's no entry, it just means the file is
> >     created in this update, so there won't be any previously recorded
> >     checksum to compare against.  If no checksum, well, for backwards
> >     compatibility we assume that no checksum always matches. */
> >  if (ent && ent->checksum)
> >    {
>
> subversion/libsvn_wc/adm_crawler.c:svn_wc_transmit_text_deltas()
> >      /* For backwards compatibility, no checksum means assume a match. */
> >     if (ent->checksum)
> >       {
> >         const char *tb = svn_wc__text_base_path (path, FALSE, pool);
> >         unsigned char tb_digest[APR_MD5_DIGESTSIZE];
>
> I think it is dangerous and propose fix version number in which text
> base checksums required.
> Next question what we should do if no checksum found: display error or
> assertion?

Since no checksum means essentially: 'invalid external input', I'd say
we need to error out. It's bad to sigabort on invalid (external)
input.

(Note: I didn't look at the patch yet.)

bye,


Erik.