On Mon, Jul 06, 2015 at 04:02:59PM +0200, Sumit Bose wrote:
On Mon, Jul 06, 2015 at 01:44:36PM +0200, Jakub Hrozek wrote:
> On Fri, Jul 03, 2015 at 05:44:27PM +0200, Pavel Reichl wrote:
> > On 07/02/2015 03:06 PM, Pavel Reichl wrote:
> > >Hello,
> > >
> > >new round of patches is attached. I discussed the required changes with
> > >Jakub to be sure we are on the same page as this thread is quite
> > >voluminous.
> > >
> > >Basically I just made the setters/getters static and removed tests of this
> > >functions. I hope that by this change we finally reached the desired level
> > >of design quality.
> > >
> > >Thanks!
> > >
> > >PS: Feel free to ignore patch 3. It's just a by-product of this
effort.
> > >
> > >
> > >_______________________________________________
> > >sssd-devel mailing list
> > >sssd-devel(a)lists.fedorahosted.org
> > >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > Attached patch set fixes minor code style issues.
>
> These patches work for me, but I also tested OTP and it doesn't behave
> as we agreed on in this thread:
>
https://www.redhat.com/archives/freeipa-users/2015-June/msg00575.html
>
> Currently only the first factor was considered during the cached auth
> period, so anything could be input as the second factor. I propose this
> patch:
>
https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/commit/?h=revie...
ACK.
CI:
http://sssd-ci.duckdns.org/logs/job/18/63/summary.html
One might argue that pam_is_authtok_cachable() can be written in a
more compact way, but imo it is the compilier's job to optimize to
code.
Yes, but I tried to keep the same style as pam_is_cmd_cachable that is
just above that.
And from the experience of debugging optimized code I know the result is
nothing like the source :)
Thank you for the review. Pushed to master:
* 7e798b94cfffe7bf8f7b477d540b95d52ca1f6e4
* 6aff93510b36799c1773d368cc218cd533c43161
* 0aa18cc0bf3447ca734476926724f1632e160807
* 32cc237aa0f3c70a4e0bc0491ec0cba0016aaf5a