You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Matthew Woehlke <mw...@users.sourceforge.net> on 2009/03/25 20:20:42 UTC
questionable code?
In find_unmerged_mergeinfo (subversion/libsvn_client/merge.c circa line
7490), the line near the bottom of the function:
if (SVN_IS_VALID_REVNUM(youngest_merged_rev))
...causes a compile error on c89 compilers. It looks like this line
should actually be:
if (SVN_IS_VALID_REVNUM(*youngest_merged_rev))
(Noticed in 1.5.6 and 1.6.0.)
--
Matthew
Please do not quote my e-mail address unobfuscated in message bodies.
--
"Yoda of Borg am I. Futile is resistance. Assimilate you, I will."
-- from http://en.wikipedia.org/wiki/Wikipedia:Yet_more_Best_of_BJAODN
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1419692
Re: questionable code?
Posted by Blair Zajac <bl...@orcaware.com>.
Paul Burba wrote:
> On Wed, Mar 25, 2009 at 6:44 PM, Blair Zajac <bl...@orcaware.com> wrote:
>> Matthew Woehlke wrote:
>>> In find_unmerged_mergeinfo (subversion/libsvn_client/merge.c circa line
>>> 7490), the line near the bottom of the function:
>>> if (SVN_IS_VALID_REVNUM(youngest_merged_rev))
>>>
>>> ...causes a compile error on c89 compilers. It looks like this line should
>>> actually be:
>>> if (SVN_IS_VALID_REVNUM(*youngest_merged_rev))
>>>
>>> (Noticed in 1.5.6 and 1.6.0.)
>> Yes, that does look like a bug. Fixed in r36783.
>>
>> Paul, should this be merged into 1.5.x and 1.6.x?
>
> Blair,
>
> Yes, I'll nominate both.
>
> It appears that this hasn't caused a problem because the only way that
> youngest_merged_rev is ever SVN_INVALID_REVNUM is if we never synced
> the reintegrate target to the reintegrate source before attempting to
> reintegrate. In that case svn_mergeinfo__filter_catalog_by_ranges()
> happily processes this bogus revision input and we simply luck out
> that it doesn't do any filtering at all and so has no impact.
>
> Regardless svn_mergeinfo__filter_mergeinfo_by_ranges should be
> checking its svn_revnum_t arguments for validity, and it is not the
> only offender on this count. I need to check through all the
> mergeinfo code and make sure SVN_INVALID_REVNUMs are not allowed to
> slip through anywhere else.
OK, good to know. I voted on both backports.
Blair
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1432664
Re: questionable code?
Posted by Paul Burba <pt...@gmail.com>.
On Wed, Mar 25, 2009 at 6:44 PM, Blair Zajac <bl...@orcaware.com> wrote:
> Matthew Woehlke wrote:
>>
>> In find_unmerged_mergeinfo (subversion/libsvn_client/merge.c circa line
>> 7490), the line near the bottom of the function:
>> if (SVN_IS_VALID_REVNUM(youngest_merged_rev))
>>
>> ...causes a compile error on c89 compilers. It looks like this line should
>> actually be:
>> if (SVN_IS_VALID_REVNUM(*youngest_merged_rev))
>>
>> (Noticed in 1.5.6 and 1.6.0.)
>
> Yes, that does look like a bug. Fixed in r36783.
>
> Paul, should this be merged into 1.5.x and 1.6.x?
Blair,
Yes, I'll nominate both.
It appears that this hasn't caused a problem because the only way that
youngest_merged_rev is ever SVN_INVALID_REVNUM is if we never synced
the reintegrate target to the reintegrate source before attempting to
reintegrate. In that case svn_mergeinfo__filter_catalog_by_ranges()
happily processes this bogus revision input and we simply luck out
that it doesn't do any filtering at all and so has no impact.
Regardless svn_mergeinfo__filter_mergeinfo_by_ranges should be
checking its svn_revnum_t arguments for validity, and it is not the
only offender on this count. I need to check through all the
mergeinfo code and make sure SVN_INVALID_REVNUMs are not allowed to
slip through anywhere else.
Paul
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1431315
Re: questionable code?
Posted by Matthew Woehlke <mw...@users.sourceforge.net>.
Blair Zajac wrote:
> Matthew Woehlke wrote:
>> In find_unmerged_mergeinfo (subversion/libsvn_client/merge.c circa line
>> 7490), the line near the bottom of the function:
>> if (SVN_IS_VALID_REVNUM(youngest_merged_rev))
>>
>> ...causes a compile error on c89 compilers. It looks like this line
>> should actually be:
>> if (SVN_IS_VALID_REVNUM(*youngest_merged_rev))
>>
>> (Noticed in 1.5.6 and 1.6.0.)
>
> Yes, that does look like a bug. Fixed in r36783.
Thanks!
> Paul, should this be merged into 1.5.x and 1.6.x?
Please do merge it into 1.6.x at least :-).
--
Matthew
Please do not quote my e-mail address unobfuscated in message bodies.
--
"Yoda of Borg am I. Futile is resistance. Assimilate you, I will."
-- from http://en.wikipedia.org/wiki/Wikipedia:Yet_more_Best_of_BJAODN
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1421263
Re: questionable code?
Posted by Blair Zajac <bl...@orcaware.com>.
Matthew Woehlke wrote:
> In find_unmerged_mergeinfo (subversion/libsvn_client/merge.c circa line
> 7490), the line near the bottom of the function:
> if (SVN_IS_VALID_REVNUM(youngest_merged_rev))
>
> ...causes a compile error on c89 compilers. It looks like this line
> should actually be:
> if (SVN_IS_VALID_REVNUM(*youngest_merged_rev))
>
> (Noticed in 1.5.6 and 1.6.0.)
Yes, that does look like a bug. Fixed in r36783.
Paul, should this be merged into 1.5.x and 1.6.x?
Regards,
Blair
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1421174