[SSSD] [PATCHES][PRELIMINARY] sudo - new design

Jan Zelený jzeleny at redhat.com
Wed Jun 27 14:28:24 UTC 2012


> > Ack to patches #0001-#0018. I haven't tested them yet though.
> > 
> > Thanks
> > Jan
> 
> I have found an issue in the timer API. I did not clear timeout after
> the request has been completed which cause SIGABRT in talloc function.
> Patches are attached.

Ack to the fix, another part of review coming:

Patch #0019: Ack
Patch #0020: Nack,
Just for safety, please test _usn for being NULL in sdap_sudo_get_usn(). Also, 
as you already discovered, there will be a segfault caused by accessing el-
>num_values because el is always NULL.

Patch #0021: Nack,
if srv_opts is NULL, you will get segfault caused by the debug statement

Patch #0022: Ack
Patch #0023: Nack,
The first part of patch #0028 should be merged with this patch otherwise it 
breaks compilation.

Patch #0024: Tentative Ack,
just one suggestion: when purging by name, I think if one fails we should just 
skip to the next one.

Patch #0025: Ack
Patch #0026: Nack,
The first part of this patch (full refresh state -> smart refresh state) should 
be merged with #0023.

Patch #0027: Nack,
First of all, I have to repeat my previous comment about the schedule label, 
please add debug messages to both condition before the label.

As per our previous discussion, please extend validations related to smart vs. 
full timer. First scenario you specifically want to handle was when both timers 
are set to zero. Also consider scenario when full refresh timeout will be 
lower than smart refresh timeout. My guess is that full refresh timeout should 
be at least 50% longer than smart refresh timeout.

Patch #0028: Ack,
but please add comment why the old enum is left there. Personally, I would 
remove it completely but I accept your reasoning about it.

Patch #0029: Nack,
you don't need the second part of the filter since if something has expiration 
timestamp the same as current time, it is as good as expired anyway. Also one 
question: do you expect sysdb_get_sudo_filter() to recieve time other than 
current time in the future? If not then I think it is not necessary to add the 
argument.

Patch #0030: Ack


Thanks
Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120627/f86e4e05/attachment.sig>


More information about the sssd-devel mailing list