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

Pavel Březina pbrezina at redhat.com
Fri Jun 14 13:00:44 UTC 2013


On 06/14/2013 02:28 PM, Jakub Hrozek wrote:
> 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;
>> +
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-subdomains-touch-krb5.conf-when-creating-new-domain-.patch
Type: text/x-patch
Size: 3907 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130614/26a4264b/attachment.bin>


More information about the sssd-devel mailing list