Lieven Govaerts wrote:
> Julian Foad wrote:
>> On Tue, 2008-06-10 at 10:59 -0400, C. Michael Pilato wrote:
>>> I seem to be doing alot of talking to myself at the moment, but
>>> that's cool.
>>
>> I'm listening. Sorry that you didn't read my mind and know that's what I
>> was doing. :-)
>>
>>> For the record, I think the correct-er of the fixes is the second patch.
>>
>> Your patch 1 (send user_args==NULL rather than uninitialised) is
>> essential to fix this regression.
>>
>> Your patch 2 ((svn_io_run_diff): Don't check user_args for NULL-ness;
>> consult the num_user_args instead) is bad: it will break this public API
>> for clients that were relying on it working the way it did work, and
>> initialising only 'user_args' but not 'num_user_args'. (Note also that
>> the test for null occurs twice in the function and your patch changes
>> only one occurrence.)
>>
>> FWIW, it looks to me like this bug was introduced in r30053 in the
>> calling code. The svn_io_run_diff() API has been stable for ages.
>>
>> The correct fix should include:
>>
>> * Your patch 1.
>>
>> * Update the doc string for svn_io_run_diff() to match the
>> implementation, which is to say that user_args must be NULL to
>> indicate no user args specified and thus get the default "-u".
>> (Presently it's ambiguous on whether user_args need be NULL
>> and/or num_user_args need be 0 to get that behaviour.)
>>
> You obviously meant:
> 3. A regression test, as this issue is important enough to hold off the
> 10th RC and almost caused a regression.
>
> Lieven - always prepared to provide test writing cycles.
The fix is in -- feel free to start your test composition now.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2008-06-10 19:05:07 CEST