Ticket31241 merged 1 #1266
Closed
Ticket31241 merged 1 #1266
Conversation
Remember that our goal in the present refactoring is to allow each subsystem to declare its own configuration structure and variables. To do this, each module will get its own config_format_t, and so we'll want a different structure that wraps several config_format_t objects. This is a "config_mgr_t".
The eventual design here will be that multiple config_format_t objects get registered with a single config_mgr_t. That config_mgr_t manages a "top-level" object, which has a pointer to the other objects. I had earlier thought of a different design, where there would be no top-level object, and config_mgr_t would deal with a container instead. But this would require a bunch of invasive refactoring that I don't think we should do just yet.
Iterating over this array was once a good idea, but now that we are going to have a separate structure for each submodule's configuration variables, we should indirect through the config_mgr_t object.
We'll want it to check all the subsidiary structures of the options object.
It's important to make sure that we don't change a config_mgr_t after we start using it to make objects, or we could get into inconsistent states. This feature is the start of a safety mechanism to prevent this problem.
We'll need to do it this way once the objects become more complex.
Every time we finalize a config manager, we now generate a new magic number for it, so that we'll get an assertion failure if we ever two to use an object with a different configuration manager than the one that generated it.
The right way to free a config object is now to wrap config_free(), always. Instead of creating an alternative free function, objects should provide an alternative clear callback to free any fields that the configuration manager doesn't manage. This lets us simplify our code a little, and lets us extend the confparse.c code to manage additional fields in config_free.
Right now, it doesn't do anything; this patch is meant to make sure that we're doing memory management correctly.
A configuration manager, in addition to a top-level format object, may now also know about a suite of sub-formats. Top-level configuration objects, in turn, may now have a suite of sub-objects.
I'm about to mess with their lists of callbacks, and I don't want to proliferate lists where we say "NULL, NULL, NULL, ..."
This is part of work to simplify the options_validate() interface
It is easier to let the validation code construct its own options if that's what you really want. This required a fair amount of changes to test_options.c: that module used to do some nutty things with partially constructed options objects.
This commit is big, but all it does is remove the now-unused "from_getconf" and "defaults" options from options_validate() and its kin.
We'll want to use this in place of direct calls to options_validate and its siblings, so that it can validate all of the configuration objects in a suite.
The old validate function is now called legacy_validate. We have
new functions for:
* Deriving fields before validation.
* validating a single configuration
* checking whether a configuration transition is allowed.
* Deriving fields after validation.
These functions can give multiple error messages; the top-level
function returns an enum explaining what went wrong.
There is a shenanigan happening here with shared_random_state, but the next commit should clean it up.
Previously the verification callback avoided checking anything, because it was only called by config_dump(), and it didn't pass on the default options. The right solution here, though, is to pass a default value to config_dump(), so that it doesn't try to validate the defaults itself.
This doesn't need to be a legacy callback.
This required us to mark a few arguments as const, but that's all to the good: validation should not modify its arguments.
Pull Request Test Coverage Report for Build 5947
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
No description provided.
The text was updated successfully, but these errors were encountered: