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