[squid-dev] [PATCH] StrList removal

[Alex Rousskov]

On 10/27/2016 08:04 AM, Amos Jeffries wrote:
> -    while ((entry = (StoreEntry *)linklistShift(&heap_walker->locked_entries))) {
> +    while (StoreEntry *entry = heap_walker->locked_entries.front()) {

This change is a bug AFAICT. Old linklistShift() makes the just-emptied
list nil, so the next call to that function will returns nil and the
loop stops. However, std::queue does not (and cannot) work like that.
The only way front() can return nil is if you push a nil pointer into
the queue which I do not think you do.

  http://en.cppreference.com/w/cpp/container/deque/front

Please use !empty() as the loop guard instead.


Rule of thumb: front() requires an !empty() guard.


> It has had several hours of use with web traffic

Rule of thumb: When testing, check that the changed code is exercised.



I have no objection to the fixed patch going in.


Thank you,

Alex.