Paul Burba wrote:
>> -----Original Message-----
>> From: Paul Burba [mailto:firstname.lastname@example.org]
>> Sent: Monday, June 25, 2007 9:26 AM
>> To: Lieven Govaerts; Mark Phippard
>> Cc: email@example.com; firstname.lastname@example.org; Daniel Rall
>> Subject: Re: svn commit: r25510 - trunk/subversion/tests/cmdline
>> Log.c:do_merged_log() - Local variable "svn_boolean_t
>> changed" is not initialized before being passed by reference
>> to check_history(). Great, except that check_history doesn't
>> always modify that arg:
>> /* Set INFO->HIST to the next history for the path *if* there
>> is history
>> * available and INFO->HISTORY_REV is equal to or greater
>> than CURRENT.
>> * *CHANGED is set to TRUE if the path has history in the
>> CURRENT revision,
>>>>>> * otherwise it is not touched. <<<<
>> * If we do need to get the next history revision for the path, call
>> * get_history to do it -- see it for details.
> Just thinking out loud, but is it really wise to have functions like
> check_history() that take a predicate argument but only sometimes
> modifies it? Doesn't seem we do that anywhere else that I can recall
> (though I haven't made an exhaustive search).
I'm not sure. check_history() has been largely unchanged since its
introduction in r14722, so it seems that we've been doing this for quite
check_history() is used to determine if any of a group of paths has been
changed in a given revision, and is called in a loop over the paths.
The reason that check_history() doesn't modify changed if the answer is
FALSE is so that previous values of TRUE don't get overwritten. In
effect, this implements a logical OR.
If updating the function, and introducing local variables in the path
loops seems like a more readable solution, feel free to do so.
Received on Mon Jun 25 16:27:35 2007