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

Sumit Bose sbose at redhat.com
Fri Sep 7 09:31:45 UTC 2012


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



More information about the sssd-devel mailing list