On Mon, Nov 25, 2013 at 7:40 PM, Jim Jagielski <[email protected]> wrote:
> > On Nov 25, 2013, at 1:09 PM, Jim Jagielski <[email protected]> wrote: > > > > > On Nov 25, 2013, at 11:53 AM, Yann Ylavic <[email protected]> wrote: > > > >> On Mon, Nov 25, 2013 at 4:53 PM, Jim Jagielski <[email protected]> wrote: > >> > >> On Nov 25, 2013, at 10:24 AM, Yann Ylavic <[email protected]> wrote: > >> > >>> > >>> As Jeff said in the other thread, I think the test here should be : > >>> if (prev_idlers <= 1) > >>> that's because dec32 was returning the new value while add32 now > returns the old one (fetch_and_sub vs sub_and_fetch). > >>> > >> > >> We aren't being consistent.. see below: > >> > >>> +static const apr_uint32_t zero_pt = ((apr_uint32_t)1 << 31); > >> > >> Hmmmm... for a 32bit int, shouldn't that be << 29 ?? > >> > >> I don't see, why 29? > >> Suppose idlers is 0: idlers - zero_pt == 0 - 2^31 == INT32_MIN; > >> and when idlers is 2^32-1: (2^32-1) - 2^31 == INT32_MAX. > >> Is there a need for more positives than negatives? > > > > All we are doing is trying to create a new offset... > > something between 0..INT32_MAX. Halfway is likely best, > > but we are talking scales here that it makes no real > > diff. > > > > In any case, ((apr_uint32_t)1 << 31) is wrong. > > Using <<30 would result in 1073741824 which is > > what we would want, if we want midway. > > > OOoooooooo. I see. You also changed idlers as well. Sometimes > it's extremely hard to make sense of the patches. Sorry about that, I should have been more verbose.
