[SSSD] [PATCH] Read and validate configuration before daemonizing
Simo Sorce
ssorce at redhat.com
Thu Sep 10 15:39:34 UTC 2009
On Thu, 2009-09-10 at 11:11 -0400, Stephen Gallagher wrote:
> Attached is a new approach to reading the configuration and exiting
> with
> an error code if the configuration is invalid. There should be no
> situation where this will affect signal/process setup anymore.
>
> Patch 0001: Simple cleanup. We're now using a private event context
> for
> the confdb and ignoring the event_ctx argument to confdb_init, so I'm
> removing it to eliminate confusion.
Ack this one.
> Patch 0002: Create the confdb and read in the configuration options
> before daemonizing so that failures can be reported immediately.
Patche is going in the right direction.
Comments inline.
> +static errno_t load_configuration(const char *config_file,
> + struct mt_ctx **monitor)
> +{
> + errno_t ret;
> + struct mt_ctx *ctx;
> + char *cdb_file = NULL;
> +
> + ctx = talloc_zero(NULL, struct mt_ctx);
Please pass in a memory context to this function, don't use NULL.
> int monitor_process_init(TALLOC_CTX *mem_ctx,
> - struct tevent_context *event_ctx,
> - struct confdb_ctx *cdb,
> + struct mt_ctx *ctx,
> const char *config_file)
> {
mem_ctx is not used anymore in this function as far as I can see,
please remove it.
> - struct mt_ctx *ctx;
> @@ -2332,6 +2343,7 @@ int main(int argc, const char *argv[])
> char *config_file = NULL;
> int flags = 0;
> struct main_context *main_ctx;
> + struct mt_ctx *monitor;
> int ret;
>
> struct poptOption long_options[] = {
> @@ -2379,16 +2391,18 @@ int main(int argc, const char *argv[])
> flags |= FLAGS_PID_FILE;
>
> /* Parse config file, fail if cannot be done */
> - ret = read_config_file(config_file);
> + ret = load_configuration(config_file, &monitor);
> if (ret != EOK) return 4;
>
> /* set up things like debug , signals, daemonization, etc... */
> ret = server_setup("sssd", flags, MONITOR_CONF_ENTRY, &main_ctx);
> if (ret != EOK) return 2;
>
> + monitor->ev = main_ctx->event_ctx;
> + talloc_steal(main_ctx, monitor);
> +
It would be nice to refactor the code to avoid this steal here, although
that may prove to be a bit complicated.
One way could be to allocate main_ctx outside of server_setup() before
load_configuration() is called, and make it not be a child of the event
context.
Although I wouldn't consider a decision not to to this as reason to
nack.
The rest looks good.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
More information about the sssd-devel
mailing list