On Thu, 2012-05-24 at 13:44 +0200, Jakub Hrozek wrote: > On Thu, May 24, 2012 at 07:17:31AM -0400, Stephen Gallagher wrote: > > On Thu, 2012-05-24 at 10:47 +0200, Jakub Hrozek wrote: > > > On Wed, May 23, 2012 at 01:42:58PM -0400, Stephen Gallagher wrote: > > > > On Wed, 2012-05-23 at 16:52 +0200, Jakub Hrozek wrote: > > > > > On Wed, May 23, 2012 at 09:03:16AM -0400, Stephen Gallagher wrote: > > > > > > If the mmap cache cannot be initialized (such as insufficient > > > > > > permissions or SELinux/AppArmor denial), we are supposed to fall > > > > > > back to > > > > > > our 1.8 behavior of LDB cache only. However, we weren't properly > > > > > > checking that the cache had been set up and we were always > > > > > > attempting to > > > > > > dereference the mmap context in fill_pwent() and fill_grent(). > > > > > > > > > > > > Fixes https://fedorahosted.org/sssd/ticket/1346 > > > > > > > > > > I think we should do the same for sss_mmap_cache_gr_store, too. And > > > > > perhaps do the check inside the functions themselves and not the > > > > > callers > > > > > so that we don't forget to add the check when we add a new call of > > > > > these > > > > > functions. > > > > > > > > How do you think we should handle the return code if we add it to the > > > > functions themselves? EOK would seem to be hiding things, but ENOMEM > > > > (insufficient space in the cache) seems a little hackish. > > > > > > I was thinking logging a MINOR_FAILURE and returning EOK. > > > > > > > Logging a minor failure would be much too noisy. We'd be polluting the > > log with an error they may not be able to fix. > > > > > But checking if the context exists in the caller is fine, too, we just > > > need to do it for both _pw_store and _gr_store. > > > > Can you point out where I missed one? My patch fixes both fill_pwent() > > and fill_grent(). Grepping through the sources shows me only those two > > locations using sss_mmap_cache_*_store(). > > You're right, you fixed both. I don't know why I didn't see that. Sorry > for the noise. > > Ack.
Pushed to master.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
