Re: [RFC PATCH 01/10] config: make config_from_gitmodules generally useful

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!