On 04/11/2018 01:26 PM, Yann Ylavic wrote:
> On Fri, Apr 6, 2018 at 10:18 AM, Yann Ylavic <ylavic....@gmail.com> wrote:
>> On Fri, Apr 6, 2018 at 8:57 AM, Ruediger Pluem <rpl...@apache.org> wrote:
>>> I don't think that a remark is strong enough here. I would like to
>>> see this parameter either added to a new apr_reslist_create_ex or if
>>> we want to stick with apr_reslist_fifo_set it should return an error
>>> if elements are already in the reslist.
>> r1828492, I kept it as a helper rather than _create_ex() because:
>> - it's already how settings seem to be added there (timeout, ...)
>> - _create() has a substantial number of args already :/
>> - _create_ex() are definitive extensions or may end up in endless
>> discussions for the next function name when/should an _ex_ex() is/be
>> needed :)
> Actually it's not that easy.
> While writing the unit test for apr_reslist_fifo_set(), I realized
> that if min > 0 is configured the reslist is immediately filled in
> upon creation, so there is no way to call apr_reslist_fifo_set()
> afterward...
> This brings us to either apr_reslist_create_ex() as you suggested, or
> disable the non-empty check in apr_reslist_fifo_set() as the original
> commit (and rely on the user to read docs...).

Other options:

1. Destroy all reslist elements when apr_reslist_fifo_set is called.
2. Reverse the order in the ring when apr_reslist_fifo_set is called.

Of course both options require a lock.

> Setting fifo just after create() with min > 0 is not an issue per se,
> it'll really matter at acquire time anyway...

I guess this is true.

> So I tend to prefer the documentation only over the _ex() (potential
> _ex++() later...), but I could live the the latter too :)
> Thoughts?



Reply via email to