Re: svn commit: r244604 - head/usr.sbin/gssd

On Sun, Dec 30, 2012 at 4:49 PM, Rick Macklem <rmacklem_at_uoguelph.ca> wrote:
> bf1783 wrote:
>> >Author: rmacklem
>> >Date: Sat Dec 22 23:21:17 2012
>> >New Revision: 244604
>> >URL: http://svnweb.freebsd.org/changeset/base/244604
>> >
>> >Log:
>> >  It was reported via email that some sshds create kerberos
>> >  credential cache files with names other than /tmp/krb5cc_<uid>.
>> >  The gssd daemon does not know how to find these credential caches.
>> >  This patch implements a new option "-s" that does a search for
>> >  credential cache files, using roughly the same algorithm as the
>> >  gssd daemon for Linux uses. The gssd behaviour is only changed
>> >  if the new "-s" option is specified. It also implements two other
>> >  new options related to the "-s" option.
>> >
>> >  Reported by: Piete.Brooks at cl.cam.ac.uk, Herbert Poeckl
>> >  Tested by: Herbert Poeckl (admin at ist.tugraz.at), Illias A.
>> >  Marinos
>> >  MFC after: 2 weeks
>>
>> ...
>>
>> >+#include <krb5.h>
>>
>> Rick:
>>
>> This breaks world built WITHOUT_KERBEROS and WITH_GSSAPI.
>>
>> Regards,
>> b.
> Could you please test the attached patch.
>
> Also, if someone who is familiar with the build/Makefile side
> of things could review this, it would be appreciated.

1. I would name WITHOUT_KERBEROS to KERBEROS_SUPPORT in the sourcefile
and CFLAGS to avoid potential confusion/noise with build logic.

2. This code should be revised per style(9):

+#else
+			fprintf(stderr, "This option not available when built"
+			    " without MK_KERBEROS\n");
+			exit(1);

In particular:

    errx(1, "This option requires Kerberos support");

Seems more succinct and addresses the actual item at hand.

3. This could be simplified as well potentially:

+.if ${MK_KERBEROS} != "no"
 DPADD=	${LIBGSSAPI} ${LIBKRB5} ${LIBHX509} ${LIBASN1} ${LIBROKEN}
${LIBCOM_ERR} ${LIBCRYPT} ${LIBCRYPTO}
 LDADD=	-lgssapi -lkrb5 -lhx509 -lasn1 -lroken -lcom_err -lcrypt -lcrypto
+.else
+CFLAGS+= -DWITHOUT_KERBEROS
+DPADD=	${LIBGSSAPI}
+LDADD=	-lgssapi
+.endif

to this:

DPADD=	${LIBGSSAPI}
LDADD=	-lgssapi
.if ${MK_KERBEROS} != "no"
CFLAGS+= -DKERBEROS_SUPPORT
DPADD+= ${LIBKRB5} ${LIBHX509} ${LIBASN1} ${LIBROKEN} ${LIBCOM_ERR}
${LIBCRYPT} ${LIBCRYPTO}
LDADD+= -lkrb5 -lhx509 -lasn1 -lroken -lcom_err -lcrypt -lcrypto
.endif

Thanks!
-Garrett