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(a)redhat.com>
>>>> To: sssd-devel(a)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(a)/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;
> +
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel