Re: [PATCH v3] submodule: prevent overwriting .gitmodules entry on path reuse

>
> K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
>
> > Add helper `submodule_active_matches_path()` so we can
> > re-implement the old “is this path already covered by
> > submodule.active?” logic without re-reading the config twice.
>
> Having duplicated code to implement what is supposed to be the same
> thing is a bug waiting to happen by them diverging from each other.
>
> Isn't the fact that our configuration reading code reads things just
> once and the caches the result good enough for the purpose of this
> code path?  Do we have a measurement that tells us that the extra
> complexity is worth the maintenance headache?
>

> > @@ -3443,7 +3452,11 @@ static int module_add(int argc, const char **argv, const char *prefix,
> >       int force = 0, quiet = 0, progress = 0, dissociate = 0;
> >       struct add_data add_data = ADD_DATA_INIT;
> >       const char *ref_storage_format = NULL;
> > +     const struct submodule *existing;
> >       char *to_free = NULL;
> > +     struct strbuf buf = STRBUF_INIT;
> > +     int i;
> > +     int allocated_sm_name = 0;
>
> A separate flag is not wrong per-se, but the idiom used in this
> project more often is to have an extra pointer variable that points
> at an allocated piece of memory (or NULL), and free the piece of
> memory unconditionally.
>
> "git grep -e to_free" to see the idiom in action.  Even better yet,
> this codepath already uses the idiom.
>
> By doing so
>
> > +     if (allocated_sm_name)
> > +             free((char *)add_data.sm_name);
>
> becomes
>
>         free(sm_name_to_free);
>
> and we can keep the "add_data.sm_name is pointing at a borrowed
> piece of memory, and we will _never_ free anything through that
> pointer" memory ownership rule.  We were borrowing from a separate
> variable sm_name_to_free, and we may free it when add_data is
> getting destroyed, or we may be borrowing from the .sm_path string,
> which we would free it when add_data is getting destroyed.
>