[SSSD] [PATCH] subdomains: touch krb5.conf when creating new domain-realm mappings

Jakub Hrozek jhrozek at redhat.com
Fri Jun 14 12:28:30 UTC 2013


On Fri, Jun 14, 2013 at 02:15:41PM +0200, Pavel Březina wrote:
> On 06/14/2013 10:03 AM, Jakub Hrozek wrote:
> >On Thu, Jun 13, 2013 at 12:20:31PM -0400, Pavel Brezina wrote:
> >>
> >>
> >>----- Original Message -----
> >>>From: "Jakub Hrozek" <jhrozek at redhat.com>
> >>>To: sssd-devel at lists.fedorahosted.org
> >>>Sent: Thursday, June 13, 2013 5:14:13 PM
> >>>Subject: Re: [SSSD] [PATCH] subdomains: touch krb5.conf when creating new domain-realm mappings
> >>>
> >>>On Thu, Jun 13, 2013 at 03:48:37PM +0200, Pavel Březina wrote:
> >>>>https://fedorahosted.org/sssd/ticket/1815
> >>>
> >>>>+errno_t sss_krb5_touch_config()
> >>>>+{
> >>>>+    const char *config = NULL;
> >>>>+    errno_t ret;
> >>>>+
> >>>>+    config = getenv("KRB5_CONFIG");
> >>>>+    if (config == NULL) {
> >>>>+        config = "/etc/krb5.conf";
> >>>>+    }
> >>>
> >>>I had a discussion with ab on IRC and the consensus was that we really
> >>>should make the location configurable. So this code works fine, we just
> >>>also need a configure-time macro to set the krb5.conf location. The
> >>>macro should default to @sysconfdir@/krb5.conf
> >>
> >>OK. Just to be clear, I should only replace "/etc/krb5.conf" with value from configure or I should remove getenv("KRB5_CONFIG") also?
> >
> >Just the hardcoded path.
> 
> Here it is.
> 

Only two nitpicks now:

> --- a/src/conf_macros.m4
> +++ b/src/conf_macros.m4
> @@ -290,6 +290,19 @@ AC_DEFUN([WITH_KRB5AUTHDATA_PLUGIN_PATH],
>      fi
>      AC_SUBST(krb5authdatapluginpath)
>    ])
> +  
> +AC_DEFUN([WITH_KRB5_CONF],
> +  [ AC_ARG_WITH([krb5_conf],
> +                [AC_HELP_STRING([--with-krb5-conf=PATH], [Path to krb5.conf file [/etc/krb5.conf]])
> +                ]
> +               )
> +
> +    KRB5_CONF_PATH="/etc/krb5.conf"

I would prefer sysconfdir used here:
KRB5_CONF_PATH="${sysconfdir}/krb5.conf"

> +    if test x"$with_krb5_conf" != x; then
> +        KRB5_CONF_PATH=$with_krb5_conf
> +    fi
> +    AC_DEFINE_UNQUOTED([KRB5_CONF_PATH], ["$KRB5_CONF_PATH"], [KRB5 configuration file])
> +  ])
>  
>  AC_DEFUN([WITH_PYTHON_BINDINGS],
>    [ AC_ARG_WITH([python-bindings],

[snip]

> --- a/src/util/sss_krb5.c
> +++ b/src/util/sss_krb5.c
> @@ -20,6 +20,7 @@
>  #include <stdio.h>
>  #include <errno.h>
>  #include <talloc.h>
> +#include <utime.h>
>  
>  #include "config.h"
>  
> @@ -1176,3 +1177,24 @@ done:
>      return ENOTSUP;
>  #endif
>  }
> +
> +errno_t sss_krb5_touch_config()

I think the definition here would read better if it used (void) explicitly
as the declaration does.

> +{
> +    const char *config = NULL;
> +    errno_t ret;
> +



More information about the sssd-devel mailing list