Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > - is it worth making struct key_value_info opaque and provide getters > for the fields so we can change the implementation in the future > without having to modify every user. We could rename it > config_context or something generic like that if we think it might > grow in scope in the future. Yes! I planned to do the key_value_info -> config_context conversion when I send the first non-RFC version for this exact reason. > - (probably impractical) could we stuff the key and value into struct > key_value_info so config_fn_t becomes > fn(const struct key_value_info, void *data) > that would get rid of all the UNUSED annotations but would mean even > more churn. Some of my colleagues also suggested this off-list. I think it is impractical for this series because I don't think anyone could reasonably review with all of the added churn. At least its current form, the churn is mostly concentrated in the signatures, but performing ^this change would make the bodies quite churny too. After this series, I think it becomes somewhat feasible with coccinelle. My .cocci files were difficult to craft because we couldn't rely on the signature of config_fn_t alone to tell us if the function is actually used as config_fn_t, but after this series, we can just use the signature since config_fn_t has a struct key_value_info param. > The advantage is that one could add functions like > kvi_bool_or_int(kvi, &is_bool) and get good error messages because > all the config parsing functions would all have access to location > information. Interesting, I hadn't considered this possibility. This seems like a pretty good abstraction to me, though I worry about the feasibility since this is yet again more churn.
