On Tue, Apr 14, 2015 at 9:51 AM, Julian Foad <julianf...@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.
