Stephen Gallagher <sgallagh(a)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