Re: [SSSD] [PATCHES] backend: initialize sudo only when it is enabled in services

[Sumit Bose]

On Wed, Sep 05, 2012 at 03:05:49PM +0200, Pavel Březina wrote:
> 0001 fixes https://fedorahosted.org/sssd/ticket/1458
> 0002 fixes a memory leak when be_process_init() fails. I think it
> should be fixed, even though the backend is in this case immediately
> terminated

Hi,

the patches compile without errors and work as expected. Nevertheless I
have a few comments:

0001:
 - I think it make sense to skip the initialization of the sudo target
   if there is no responder configured. But to avoid surprises on the
   user side I would recommend to do the initialization if it is
   configured explicitly in sssd.conf, i.e sudo_provider = ldap, and
   give a warning in the logs that the corresponding responder will not
   be running.
> ret = confdb_get_string(ctx->cdb, ctx, CONFDB_MONITOR_CONF_ENTRY,
>                        CONFDB_MONITOR_ACTIVE_SERVICES, NULL, &services);
> if (ret != EOK) {
>    DEBUG(SSSDBG_FATAL_FAILURE,
>          ("Unable to read from confdb [%d]: %s\n", ret, strerror(ret)));
>    return ret;
> }

- I would prefer to use confdb_get_string_as_list() and use strcmp()
   later on, to be on the safe side.

0002: ACK

bye,
Sumit