[SSSD] [PATCH] Backward GOTOs rewritten into do-while loops.

Jakub Hrozek jhrozek at redhat.com
Wed Aug 8 16:10:49 UTC 2012


On Wed, Aug 08, 2012 at 09:24:50AM -0400, Simo Sorce wrote:
> Ack from me, however if you do not like the if statement inside the loop
> you can also change it to be:
> 
> while (true) {
> status = ....
> if (status != NSS_STATUS_TRYAGAIN) {
> break;
> }
> ....
> }
> 

Ondrej, please let me know whether you're going to make this change or
not. I don't really mind personally, both forms are fine from my point
of view.

The patch itself works good, but unfortunately when I was testing it I
found that the proxy provider is broken (and was even before your
patch):

https://fedorahosted.org/sssd/ticket/1466
https://fedorahosted.org/sssd/ticket/1467

> Also looking at this loop I see a potential for infinite loop in case
> something goes wrong and the internal call keeps returning
> NSS_STATUS_TRYAGAIN.
> If we want to address that issue, we may want to change the condition to
> exit the loop to be a time based condition, something like:
> 
> enter_time = time(NULL);
> 
> while (enter_time + timeout < time(NULL) {
> status = ....
> if (status != NSS_STATUS_TRYAGAIN) {
> break;
> }
> ....
> }
> 
> if status == NSS_STATUS_TRYAGAIN -> error
> 
> 
> I would like to see this (maybe in a separate patch on top of your
> current patch, as the proxy provider is synchronous already so not
> allowing it to loop for too long is a necessary condition to avoid
> having the monitor kill it.
> 
> But I would like to know what other think about this. A very slow nss
> module may end up having it's call killed midair ...
> 

In general this is true for any operation. Marko was hitting a bug with
an extremely large AD deployment where saving data to sysdb took so long
the back end got killed..

But I tend to agree, as far as I can tell, this is the only place in the
SSSD where we perform both network IO and disk IO synchronously in a
loop.

Care to open a ticket?



More information about the sssd-devel mailing list