subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
Subject Re: Issue #4554, Wrong file length with PLAIN representations in FSFS
Date Tue, 14 Apr 2015 15:38:51 GMT
On Tue, Apr 14, 2015 at 9:51 AM, Julian Foad <julianfoad@gmail.com> wrote:

> Stefan Fuhrmann wrote:
> > Julian Foad wrote:
> >> r1654932 introduced into svn_fs_fs__file_length():
> >>
> >>   if data_rep->expanded_size is 0:
> >>     /* ... A plain representation may specify its EXPANDED LENGTH as "0"
> >>          in which case, the SIZE value is what we want.
> >>
> >>          Because EXPANDED_LENGTH will also be 0 for empty files, while
> >>          SIZE is non-null, we need to check wether the content is
> >>          actually empty.  ... */
> >>       if data_rep->md5 == empty_md5:
> >
> > if (memcmp(digest,empty_digest))
> > ... which is the opposite condition.
>
> Oops, yes.
>
> My two concerns below still stand.
>
>         if data_rep->md5 != empty_md5:
>
> >>           *length = data_rep->size
> >>       else:
> >>           *length = 0
> >>
> >> But how could SIZE be non-zero for an empty file? (If we were allowed
> >> to store the rep as a delta from <non-empty> to <empty>, that would
> >> have SIZE != 0, but a delta rep is not allowed when the (true)
> >> expanded size is zero, according to the 'structure' document.)
>

Now I see. That is a documentation snafu. Fixed in r1673445.
What it meant is that for DELTA reps the special casing did
never apply. A self-delta of an empty rep is valid, though.


> >> This code is only in svn_fs_fs__file_length(). It seems to me that
> >> other places where the ->expanded_size field is used, such as
> >> svn_fs_fs__file_text_rep_equal(), are also affected by the problem.
> >> This would falsely return "they're equal" if one or both reps have
> >> ->expanded_size==0 for a non-empty file.
>

Working on that ;)

-- Stefan^2.

Mime
View raw message