On Thu, 2010-10-28 at 15:52 +0100, Julian Foad wrote: > Daniel Shahaf wrote: > > +1 > > > > stef...@apache.org wrote on Wed, Oct 27, 2010 at 21:23:35 -0000: > > > URL: http://svn.apache.org/viewvc?rev=1028104&view=rev > > > Log: > > > Adapt string unit test to recent behavioral changes. > > > > > > * subversion/tests/libsvn_subr/string-test.c > > > (test10): relax tests on string capacity > > Sure, +1 to your changes here. > > But what are the rest of these crazy "requirements" in this old test? > > [...] > > > /* Test that: > > > - * - The initial block was just the right fit. > > > + * - The initial block was at least the right fit. > > > + * - The initial block was not excessively large. > > Yup, great. > > > > * - The block more than doubled (because second string so long). > > This works, for typical alignments and this test data. But "more than > doubled" is not necessary. A sensible test would be that it "at least > doubled". > > > > * - The block grew by a power of 2. > > Why would we care whether it grew by a power of 2? Any growth by at > least a factor of 2 is efficient and satisfactory. > > > > */ > > > - if ((len_1 == (block_len_1 - 1)) > > > - && ((block_len_2 / block_len_1) > 2) > > > - && (((block_len_2 / block_len_1) % 2) == 0)) > > > + if ((len_1 <= (block_len_1 - 1)) > > > + && ((block_len_1 - len_1) <= APR_ALIGN_DEFAULT(1)) > > > + && ((block_len_2 / block_len_1) > 2) > > > + && (((block_len_2 / block_len_1) % 2) == 0)) > > That last line does NOT check that the block length grew by a power of > two anyway. It checks it grew by a factor of [2 to 2.9999] or [4 to > 4.9999] or [6 to 6.9999] or ... > > Let's axe that last line.
Done in r1028340. Apologies in advance for the merge conflict this will cause when merging your change to trunk. - Julian
