diff --git a/cf-reactor/file_watcher.c b/cf-reactor/file_watcher.c index b44730355ea..fa4f6d54809 100644 --- a/cf-reactor/file_watcher.c +++ b/cf-reactor/file_watcher.c @@ -82,7 +82,7 @@ typedef struct } FileWatcherState; -void *FileWatcherStateNew(const Rlist *args) +void *FileWatcherStateNew(ARG_UNUSED const EvalContext *ctx, const Rlist *args) { assert(args != NULL); const char *path = RlistScalarValue(args); @@ -93,7 +93,7 @@ void *FileWatcherStateNew(const Rlist *args) return fws; } -void *FileWatcherSizeStateNew(const Rlist *args) +void *FileWatcherSizeStateNew(ARG_UNUSED const EvalContext *ctx, const Rlist *args) { assert(args != NULL && args->next != NULL); const char *path = RlistScalarValue(args); diff --git a/cf-reactor/file_watcher.h b/cf-reactor/file_watcher.h index b184d3651f0..9c426883da7 100644 --- a/cf-reactor/file_watcher.h +++ b/cf-reactor/file_watcher.h @@ -29,8 +29,8 @@ #include /* WatcherCheckResult */ -void *FileWatcherStateNew(const Rlist *args); -void *FileWatcherSizeStateNew(const Rlist *args); +void *FileWatcherStateNew(const EvalContext *ctx, const Rlist *args); +void *FileWatcherSizeStateNew(const EvalContext *ctx, const Rlist *args); void FileWatcherStateDestroy(void *state); WatcherCheckResult FileWatcherCheckFileDeleted(void *state); diff --git a/cf-reactor/reactor_transform.c b/cf-reactor/reactor_transform.c index 565e5a1094f..23650035ef8 100644 --- a/cf-reactor/reactor_transform.c +++ b/cf-reactor/reactor_transform.c @@ -132,11 +132,13 @@ static const Rlist *GetThenBundles(const Promise *pp) /*****************************************************************************/ /* A constraint of the 'when' body passed to the watcher of an event, with the - * type of value it accepts */ + * type of value it accepts. An absent optional constraint isn't passed, so + * optional ones must come after the required ones. */ typedef struct { const char *name; RvalType type; + bool optional; } EventArg; typedef struct @@ -151,13 +153,13 @@ typedef struct static const EventArg NO_EXTRA_ARGS[] = { - { NULL, RVAL_TYPE_NOPROMISEE } + { NULL, RVAL_TYPE_NOPROMISEE, false } }; static const EventArg FILE_SIZE_EXCEEDS_ARGS[] = { - { REACTOR_SIZE_THRESHOLD, RVAL_TYPE_SCALAR }, - { NULL, RVAL_TYPE_NOPROMISEE } + { REACTOR_SIZE_THRESHOLD, RVAL_TYPE_SCALAR, false }, + { NULL, RVAL_TYPE_NOPROMISEE, false } }; #define EventTypeNew(name, type, args, new_state, destroy_state, check) \ @@ -264,8 +266,9 @@ static bool EventsPromiseGetInterval(const Promise *pp, time_t *interval) return true; } -static bool ResolveWhenBody(const Promise *pp, WatcherOptions *opt) +static bool ResolveWhenBody(const EvalContext *ctx, const Promise *pp, WatcherOptions *opt) { + assert(ctx != NULL); assert(pp != NULL); assert(opt != NULL); // Get the event and its arg @@ -279,9 +282,20 @@ static bool ResolveWhenBody(const Promise *pp, WatcherOptions *opt) } // If the event has additional args, add them to the list of args + NDEBUG_UNUSED bool seen_optional = false; for (const EventArg *extra_arg = event->extra_args; extra_arg->name != NULL; extra_arg++) { + /* The state constructors read args by position. So optional args + * must come last: an absent one then only shortens the list, instead + * of shifting the args after it. */ + assert(!seen_optional || extra_arg->optional); + seen_optional = seen_optional || extra_arg->optional; + const Constraint *con = PromiseGetConstraint(pp, extra_arg->name); + if (con == NULL && extra_arg->optional) + { + continue; + } if (con == NULL) { Log(LOG_LEVEL_ERR, @@ -336,10 +350,15 @@ static bool ResolveWhenBody(const Promise *pp, WatcherOptions *opt) return false; } - opt->state = event->new_state(args); + opt->state = event->new_state(ctx, args); + RlistDestroy(args); + if (opt->state == NULL) + { + /* The state constructor logs why */ + return false; + } opt->check_callback = event->check; opt->destroy_state = event->destroy_state; - RlistDestroy(args); return true; } @@ -364,7 +383,7 @@ static PromiseResult KeepEventsPromise(EvalContext *ctx, const Promise *pp) } WatcherOptions opt; - if (!ResolveWhenBody(pp, &opt)) + if (!ResolveWhenBody(ctx, pp, &opt)) { return PROMISE_RESULT_FAIL; } diff --git a/cf-reactor/watcher.h b/cf-reactor/watcher.h index bf12a91f63c..46f1ef6c137 100644 --- a/cf-reactor/watcher.h +++ b/cf-reactor/watcher.h @@ -35,7 +35,11 @@ typedef enum WATCHER_CHECK_ERROR, /* unable to tell whether the event happened */ } WatcherCheckResult; -typedef void *(*WatcherStateNewFn)(const Rlist *args); +/* Called on the main thread when the policy is evaluated. ctx must not be + * kept in the state: the EvalContext is not thread safe, so the watcher + * thread can't use it. Returns NULL, after logging why, if the state can't + * be created. */ +typedef void *(*WatcherStateNewFn)(const EvalContext *ctx, const Rlist *args); typedef WatcherCheckResult (*WatcherCheckFn)(void *state); typedef void (*WatcherStateDestroyFn)(void *state); diff --git a/tests/unit/file_watcher_test.c b/tests/unit/file_watcher_test.c index 51bdb0ccf4f..9ff463f88bb 100644 --- a/tests/unit/file_watcher_test.c +++ b/tests/unit/file_watcher_test.c @@ -28,7 +28,7 @@ static void *NewFileDeletedState(const char *path) { Rlist *args = NULL; RlistAppendScalar(&args, path); - void *state = FileWatcherStateNew(args); + void *state = FileWatcherStateNew(NULL, args); RlistDestroy(args); return state; } diff --git a/tests/unit/watcher_test.c b/tests/unit/watcher_test.c index c297793698f..b74308b8901 100644 --- a/tests/unit/watcher_test.c +++ b/tests/unit/watcher_test.c @@ -30,7 +30,7 @@ static WatcherOptions *FileDeletedOptions(const char *path, time_t interval) Rlist *args = NULL; RlistAppendScalar(&args, path); opt = (WatcherOptions) { - .state = FileWatcherStateNew(args), + .state = FileWatcherStateNew(NULL, args), .check_callback = FileWatcherCheckFileDeleted, .destroy_state = FileWatcherStateDestroy, .interval = interval,