On 2018/10/25 11:18:51, Branko Čibej <br...@apache.org> wrote:
> On 25.10.2018 12:41, BURT, Matthew J wrote:
> >
> > Hi,
> >
> >
> >
> > We’ve got a number of developers using the javahl bindings from Eclipse
> >
> > via subclipse.
> >
> >
> >
> > One of them recently reported a coredump against 1.10.3. I’ve also
> >
> > reproduced it with 1.9.7.
> >
> >
> >
> > The problem occurs when SVNClient::diff() is called with a diff-cmd
> >
> > defined in the user’s ~/.subversion/config file.
> >
> >
> >
> > SVNClient::diff() calls svn_client_diff6() with the errstream explicitly
> >
> > set to NULL and the diff command in dwi->diff_cmd.
> >
> >
> >
> > Further down in the code, if diff_content_changed() from
> >
> > subversion/libsvn_client/diff.c is called, the NULL errstream is
> >
> > de-referenced with a call to svn_stream__aprfile().
> >
> >
> >
> > I’ve put together a simple patch to avoid the null dereference, but I’m
> >
> > not 100% sure this is the correct solution. The code suggests to me that
> >
> > the intention is to allow the errstream parameter for svn_client_diff6()
> >
> > to be set to NULL, but maybe I’m misreading it.
> >
> >
> >
> > Is this the correct approach, and should I submit this patch for
> >
> > consideration?
> >
>
> The docstring for svn_client_diff7 (..._diff6 was deprecated on trunk)
> doesn't say that outstream or errstream may be NULL; that's pretty
> definitive. This should be fixed in the native JavaHL code; either an
> actual stream should be used such that JavaHL users can read it (in this
> case I suspect that the JavaHL API would have to be upgraded), or a
> stream that ignores all writes should be used.
>
> -- Brane
>
>
OK - that's clear.
I've replaced both NULL values in the JavaHL code with an svn_stream_empty in
the pool that's in-scope for the call. We've re-tested with Eclipse/subclipse
and it seems to have solved the crash we're seeing. This is on 1.10.3.
I can submit a patch for trunk if you like, but I note that JavaHL on trunk is
still using ..._diff6.
