Hi guys,

while we discuss the other bug, I have another question:
In the "update_required()" call, we have the following check:


         /*
          *    If we've opened in the last 10 minutes, then
          *    open rather than update.
          */
         if ((now - server->last_failed_open) > 600) {
             return false;
         }

Now, this seems to do exactly the opposite as stated in the comment. If 
the server failed in the last 10 minutes (that is, (now - 
server->last_failed_open) <= 600), then the code returns true (well, it 
does not return false, but this is the last check so it will indeed 
return true). True here means update. So probably the comment needs to 
be updated.

The other problem I see is: What happens if the error happened more than 
10 minutes ago? It seems that "update_required()" will return false, and 
the REALM will never be updated (leading to user authentication errors 
with no reason). I know this is unlikely since we probably will try to 
query for a realm again within that 10 minutes, but it seems to be 
possible under certain circumstances.

If the purpose of the code was to check whether there was an error and 
update the REALM in such a case, why not using the following check instead?

         /*
          *    If the REALM did not fail, open rather than update
          */
         if (server->last_packet_recv > server->last_failed_open){
             return false;
         }

Unless I'm wrong, this code seems to have a similar effect but without 
having the 10 minute limitation (note that when a realm is created, 
last_packet_recv is set to the current time, so at creation time, it 
will always be > than last_failed_open).

Regards,
Alejandro