openSUSE / libpathrs

C-friendly API to make path resolution safer on Linux.
GNU Lesser General Public License v3.0
66 stars 5 forks source link

capi: add a configuration interface #21

Open cyphar opened 4 years ago

cyphar commented 4 years ago

The current pathrs_configure is designed after Linux's extensible syscall semantics. However, it seems to me that a string-based configuration setup would be far easier to extend naturally. Maybe the value should be an opaque pointer or some kind of typed-union struct, but having string keys would make extensibility so much simpler as well as lending itself very well to configuration files.

cyphar commented 4 years ago

My current idea looks something like:

pathrs_config_set(PATHRS_ROOT, root, "resolver.type", "emulated");
pathrs_config_set(PATHRS_NONE, NULL, "error.backtraces", true);

But the main problem is that you'd have to assume the type of the argument in this case (which is pretty annoying and might even be a bad idea with Rust's memory safety requirements). The alternative is something more like

pathrs_config_set(PATHRS_ROOT, root, "resolver.type", PATHRS_STRING, "emulated");
pathrs_config_set(PATHRS_NONE, NULL, "error.backtraces", PATHRS_BOOL, true);

Unfortunately that's just ugly. libconfig gives us a neat alternative:

pathrs_config_set_string(PATHRS_ROOT, root, "resolver.type", "emulated");
pathrs_config_set_boolean(PATHRS_NONE, NULL, "error.backtraces", true);

But then you run into the fun case of fetching the configuration values:

bool boolean;
char *str;
pathrs_config_get_boolean(PATHRS_NONE, NULL, "error.backtraces", &boolean);
pathrs_config_get_string(PATHRS_ROOT, root, "resolver.type", &str);
// how do we de-allocate str? do we need a pathrs_free(PATHRS_STRING, str)?

Anyway, I think the libconfig approach is the "nicest" one but we still need to figure out how the Rust API for configuration will work (which is an entirely separate problem -- see #24).

cyphar commented 1 week ago

Now that we redesigned the API in #34, the config API probably will need to look like:

pathrs_config_t *conf = pathrs_config_new();
if ((liberr = pathrs_config_set_bool(conf, "errors.simple_errno", true)) < 0)
    goto err;
if ((liberr = pathrs_config_set_string(conf, "root.resolver", "opath")) < 0)
    goto err;

pathrs_resolve(rootfd, "foo"); // no config
pathrs_resolve_c(conf, rootfd, "foo"); // with the config
pathrs_config_free(conf);

While all of the stuff we'd want would work as bitflags at the moment, I'm a little worried about future extensibility.