OpenIDC/mod_auth_openidc

Checking for dead memcache server buggy?

rpluem-vf opened this issue · 1 comments

In

rc = rc && (context->cache_memcache->live_servers[0]->status != APR_MC_SERVER_DEAD);

doesn't it need to be

rc = rc && (context->cache_memcache->live_servers[i]->status != APR_MC_SERVER_DEAD); 

because otherwise I don't see how we iterate over all servers as the loop body in

for (i = 0; rc && i < context->cache_memcache->ntotal; i++)
rc = rc && (context->cache_memcache->live_servers[0]->status != APR_MC_SERVER_DEAD);

is invariant as neither i is used nor the array pointer of live_servers gets incremented.

Furthermore, what should be tested? That all servers are up? In this case the check would be correct, but I guess we would like to check that a least one server is up and hence the check would need to be:

	int rc = FALSE;
	int i;
	for (i = 0; !rc && i < context->cache_memcache->ntotal; i++)
		rc = rc || (context->cache_memcache->live_servers[i]->status != APR_MC_SERVER_DEAD);
	return rc;

or

	int i;
	for (i = 0;  i < context->cache_memcache->ntotal; i++) {
		if (context->cache_memcache->live_servers[i]->status != APR_MC_SERVER_DEAD)
		    return TRUE;
        }
	return FALSE;