Re: Segmentation fault during diff generation

Peter Galcik wrote:
> I am using SVN on Solaris platform (svn, version 1.8.5 [...]) and Windows
> (svn, version 1.8.10 [...]) with changelist feature.
>
> Normal svn diff command without changelist go well but when valid changelist is passed to command, crash occurs.

Hi Peter. Thank you for this report.

I can reproduce the bug on Ubuntu with Subversion 1.8.0 and 1.8.10 and trunk_at_1621760. The command I used, in a Subversion trunk checkout, was:

svn diff ^/subversion/trunk . --changelist tmp

The bug is in svn_wc__diff_base_working_diff():

  if (changelist_hash && !svn_hash_gets(changelist_hash, changelist))
    return SVN_NO_ERROR;

which crashes when processing a file whose 'changelist' is null. This should be:

  if (changelist_hash
      && (!changelist || !svn_hash_gets(changelist_hash, changelist)))
    return SVN_NO_ERROR;

On fixing that, I noticed the diff still showed differences even though my WC has no nodes assigned to changelist 'foo'. I had some locally-added files and directories in my WC, and it showed diffs for them. The next bug is in svn_wc__diff_local_only_file() where it says:

  if (changelist && changelist_hash
      && !svn_hash_gets(changelist_hash, changelist))

which skips a locally added file that is in the wrong changelist but fails to skip one that is in no changelist. The condition should be the same as in svn_wc__diff_base_working_diff().

Then there is a bug that causes it to show property changes for added directories. More work is needed. I haven't found this one or checked all the other permutations or code paths here.

We need tests for this.

We could of course first make a partial fix just to avoid crashing, on the grounds that crashing is more serious than wrong output, but let's see if we can put in a little more effort and fix the behaviour as well.

Are you (Peter) or anyone else willing and able to help, perhaps starting with creating a test to add to subversion/tests/cmdline/diff_tests.py? No worries if not.

(In the longer term, I would like it if we could implement this changelist filtering in one place instead of scattering these "if" conditions around. I would like to see a code structure more analogous to "find /wc/path <node selection conditions: depth, changelists, etc.> | xargs diff", probably using the existing svn_wc__internal_walk_children() function or something similar to it.)

- Julian
Received on 2014-09-01 15:59:25 CEST