subversion-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko Čibej <br...@apache.org>
Subject Re: Coredump when javahl SVNClient::diff() called with diff-cmd set
Date Mon, 29 Oct 2018 12:15:39 GMT
On 29.10.2018 10:20, matthew.burt@nats.co.uk wrote:
>
> On 2018/10/25 11:18:51, Branko Čibej <brane@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.

Please do submit a patch against trunk. That JavaHL has not been updated
to ..._diff7 is not your problem. :)

-- Brane


Mime
View raw message