[SSSD] [PATCH] Conditionalize support for DIR cache in the Kerberos provider

Jan Zeleny jzeleny at redhat.com
Fri Jun 15 17:53:43 UTC 2012


Stephen Gallagher <sgallagh at redhat.com> wrote:
> On Fri, 2012-06-15 at 11:25 -0400, Stephen Gallagher wrote:
> > On Fri, 2012-06-15 at 17:16 +0200, Jan Zelený wrote:
> > > > We only support the DIR cache on Kerberos 1.10 and higher. We need to
> > > > make sure we still build and run on older systems.
> > > > 
> > > > Patch 0001: Minor fix for building on little-endian RHEL 5 systems.
> > > > (Building for ppc was broken)
> > > > 
> > > > Patch 0002: Conditionalize DIR cache
> > > 
> > > Couple comments:
> > > 
> > > I see following warnings during compilation:
> > > src/providers/krb5/krb5_utils.c: In function 'find_ccdir_parent_data':
> > > src/providers/krb5/krb5_utils.c:246:72: warning: declaration of
> > > 'dirname' shadows a global declaration [-Wshadow]
> > > src/providers/krb5/krb5_utils.c: In function 'create_ccache_dir':
> > > src/providers/krb5/krb5_utils.c:335:31: warning: declaration of
> > > 'dirname' shadows a global declaration [-Wshadow]
> > > src/providers/krb5/krb5_utils.c: In function 'create_ccache_dir_head':
> > > src/providers/krb5/krb5_utils.c:531:11: warning: declaration of
> > > 'dirname' shadows a global declaration [-Wshadow]
> > 
> > These were there before my changes. They're mostly harmless, but we
> > should fix them eventually. Not relevant to this patch.
> > 
> > > in the patch #0002 there are some places in the patch where you have
> > > lines with SSS_KRB5_TYPE_DIR ifdefed and other places where it is
> > > outside the ifdef. Could you please fix that so it is everywhere the
> > > same?
> > 
> > Will do. I was originally thinking I'd just have them explicitly fall
> > into the default: case, but I suppose it makes more sense to just ifdef
> > it completely.
> > 
> > > Other than that everything compiles fine. However when I try to run it,
> > > sssd doesn't start and following message is in the log file:
> > > 
> > > sssd: Cannot load configuration database
> > > 
> > > Not sure if it's related but I started with clean setup, cache and logs
> > > deleted. Test suite works fine.
> > 
> > This was a bug in your sssd.conf. You had the pac responder enabled, but
> > this build doesn't have it, so it was throwing an error.
> > 
> > Testing is ongoing.
> 
> All changes requested above have been made, including avoiding the
> shadowing (in new patch 0003).
> 
> I've completed my testing to my satisfaction with no additional changes
> except those that Jan requested in his review.

Nack,
there is one more warning that you missed:

src/providers/krb5/krb5_utils.c: In function ‘get_cc_be_ops’:
src/providers/krb5/krb5_utils.c:570:5: warning: enumeration value 
‘SSS_KRB5_TYPE_DIR’ not handled in switch [-Wswitch]

Also please use new debug levels in your patch #0003


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/20120615/a3fc5665/attachment.sig>


More information about the sssd-devel mailing list