> OK, the fact I was overlooking was that the "config_fn_t" argument > passed to config_from_gitmodules is what we are actually worried about, > it's the config callback which could allow generic config in .gitmodules > to sneak in. That is the precise point that I was trying to communicate. :) > > So, from Brandon's message I derive that using the gitmodules_cb from > submodules-config.c as a callback would be safe, as that's what > repo_read_gitmodules already uses anyways, and it only allows submodules > configuration. I agree. > > However, what about avoiding exposing the dangerous interface altogether > and adding ad-hoc helpers when needed? > > I mean: > > 0. Move config_from_gitmodules to submodule-config.c as per Bradon's > suggestion. That sounds good to me. > 1. Add public helpers in submodule-config.[ch] to handle backwards > compatibility, like: fetch_config_from_gitmodules() and > update_clone_config_from_gitmodules() these would be used by fetch > and update-clone function and would not accept callbacks. > > This would mean moving the callback functions > gitmodules_fetch_config() and gitmodules_update_clone_config() into > submodule-config.c and making them private. The helpers will call > config_from_gitmodules() with them. > > 2. Now that config_from_gitmodules it's not used in the open it can be > made private too, so it can only be used in submodule-config.c > > 3. Use config_from_gitmodules in repo_read_gitmodules as the > gitmodules_cb function should be safe to use as a config callback. That sounds all good to me. > > 4. Add a new gitmodules_get_value() helper in submodule-config.c which > calls config_from_gitmodules with a "print" callback and use that > helper for "submodule--helper config", > > 5. At this point we shouldn't worry too much about the .gitmodules > content anymore, and we can possibly extend config_from_gitmodules > to read from other locations like HEAD:.gitmodules. These two would actually align with your goal of the original series. :) > > This way the number of users of config_from_gitmodules remains strictly > controlled and confined in submodule-config.c > > I know, we could end up adding more code with the helpers but that could > be justified by the more protective approach: we would be using symbols > scoping rules instead of comments to ensure something. > > If you think this may be worth a shot I can send a series which covers > items from 0 to 3. That would be great! Thanks, Stefan > > Ciao, > Antonio > > P.S. Always relevant: https://youtu.be/8fnfeuoh4s8 (I swear it's not > Rick Astley) heh!
