On Thu, Jun 07, 2012 at 11:47:35AM +0200, Jan Zelený wrote:
> > On Thu, May 31, 2012 at 09:17:18PM +0200, Jan Zeleny wrote:
> > > At this moment we will support only asterisk, designating "all
> > > services".
> > >
> > >
https://fedorahosted.org/sssd/ticket/1360
> > >
> > > Thanks
> > > Jan
> >
> > Nack, you need to initialize services to NULL, otherwise if any
> > operation before the strdup failed, you would free random pointer.
>
> Good catch, fixed.
>
> > You can also use sizeof(ALL_SERVICES)-1 and avoid defining
> > ALL_SERVICES_LEN (and be safe if ALL_SERVICES changed, not that it's
> > likely).
>
> As you have said, the constant is not likely to be changed. The next
> planned change is not to use the constant at all and rather dynamically
> fill this in by PAM responder. Hence I think leaving this is ok.
>
> > Does the SELinux feature work at all without the patch? If not, we
> > should consider moving the ticket to 1.8
>
> The original documentation for the feature was incorrect so it didn't
> work (or rather it did work but pam_selinux got all confused). This time
> I tested the feature and it is working. There are still some rough edges
> but I suppose they are a part of intended behaviour of pam_selinux.
>
> To sum up, yes, this should be also backported to 1.8. Patch attached and
> tested.
Sorry, one more thing I didn't notice before, can you move zeroing the
errno right before sss_atomic_write_s() ?