> -----Original Message----- > From: Bert Huijben [mailto:b...@qqmail.nl] > Sent: donderdag 20 oktober 2011 16:48 > To: 'Julian Foad'; 'Paul Burba'; dev@subversion.apache.org > Subject: RE: svn commit: r1181090 - Follow-up to r1180154: > svn_rangelist_merge2 optimization. > > > > > -----Original Message----- > > From: Julian Foad [mailto:julian.f...@wandisco.com] > > Sent: donderdag 20 oktober 2011 16:45 > > To: Paul Burba; dev@subversion.apache.org > > Subject: Re: svn commit: r1181090 - Follow-up to r1180154: > > svn_rangelist_merge2 optimization. > > > > Hi Paul. > > > > It looks like we can simplify this by omitting the first branch of the > > if ... else ... else construct: > > > > [[[ > > if (delete_index == (arr->nelts - 1)) > > { > > /* Deleting the last or only element in an array is easy. */ > > apr_array_pop(arr); > > } > > + else if ((delete_index + elements_to_delete) == arr->nelts) > > + { > > + /* Delete the last ELEMENTS_TO_DELETE elements. */ > > + arr->nelts -= elements_to_delete; > > + } > > else > > ]]] > > > > Or is there some reason why 'apr_array_pop' is preferred in the N==1 > case? > > The first case is plain wrong. (And I should have noticed when I first > reviewed > this function) > > It doesn't handle elements_to_delete for N > 1.
But if the index is that last element, you can only remove one element, so it really depends on how we handle out of range values. > So +1 on removing that branch. This stands > > Bert
