This is with r9926 of Subversion trunk:
$ svnadmin create foo
$ svn mkdir -m "Make a directory." file://`pwd`/foo/x
$ svn propdel --revprop -r1 svn:log file://`pwd`/foo/
Segmentation fault
$
Tracing it down in GDB, we see in subversion/libsvn_repos/hooks.c, in the
function create_temp_file(), that the parameter 'value' is NULL. Since the
function accesses value->len and value->data, that's a problem.
The picture gets a little bit more complicated as we trace up the stack.
create_temp_file() is called from svn_repos__hooks_pre_revprop_change(), which
claims in its doc string that the VALUE parameter is the old, unchanged value of
the prop:
/* Run the pre-revprop-change hook for REPOS. Use POOL for any
temporary allocations. If the hook fails, return
SVN_ERR_REPOS_HOOK_FAILURE.
REV is the revision whose property is being changed.
AUTHOR is the authenticated name of the user changing the prop.
NAME is the name of the property being changed.
VALUE is the current (unchanged) value of the property. */
svn_error_t *
svn_repos__hooks_pre_revprop_change (svn_repos_t *repos, ...
But go up one more stack frame and we're in svn_repos_fs_change_rev_prop(),
which does some very strange stuff. It takes the *new* property value as a
parameter, and fetches the old value from the filesystem directly. So now it
has both in its hands... And what does it do?
It passes the *new* value to svn_repos__hooks_pre_revprop_change(), and the
*old* value to svn_repos__hooks_post_revprop_change(). Of course, we never get
to the latter call, because we fail in the former.
Clearly this defies svn_repos__hooks_pre_revprop_change()'s documentation, but I
think it's the doc string that's wrong. The way this system works, IIRC, is
that each hook needs to be passed the propval that it can't obtain simply by
asking the filesystem. For the pre-revprop-change hook, that's the new value,
and for the post-revprop-change, it's the old value.
(Note that svn_repos__hooks_post_revprop_change()'s doc string is consistent
with this design.)
I think a little more investigation and thought is needed here than I can give
right now, that's why I'm filing an issue. Here's the problem:
Our whole revprop change hook system appears to be in a half-finished state...
Yet I can't find any of the open issues about its completion that I remember
filing N years ago. This all started with a simple problem, namely that 'svn
propedit --revprop -rXXX svn:log' doesn't show a diff, but instead just shows
the new property value. We'd like for it to show diffs instead, and there were
two ways to do it:
1. The pre-revprop-change hook and post-revprop-change hook could
cooperate in complex and hard-to-maintain ways
2. The post-revprop-change hook could somehow get the old value, to
diff against.
We much preferred (2), and symmetrically, it seemed like a good idea for the
pre-revprop-change to get the *new* value, as well as for post- to get the old
value. That way you could compare old and new both before and after the change.
It appears now that pre-revprop-change is getting, or trying to get, the new
value on stdin nowadaways (I say "trying to" because there's a bug in the
implementation of that feature, hence this issue).
But there's no corresponding change to the post-revprop-change system!
True, svn_repos__hooks_post_revprop_change() does take a parameter OLD_VALUE.
But it is never used; there's just a "###" comment in the code about how we
should pass it to the hook. Is there some reason we're not doing so, in the
same way that we do in svn_repos__hooks_pre_revprop_change()?