Hi,
On Sun, 25 Jan 2009, Lars Hjemli wrote:
> On Sun, Jan 25, 2009 at 12:57, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
>
> > My reasoning for "*" instead of "all" and "" instead for "checkedout"
> > was that you could allow "<name1>,<name2>" at some stage, where <name>
> > would first be interpreted as a submodule group, and if that fails, as
> > submodule name.
> >
> > Thinking about that more, "" seems illogical, that should rather mean
> > "none", i.e. the same as --no-submodules. The "checkedout" could be
> > "." then, perhaps? As in "what we have checked out in ./, the current
> > directory"?
>
> Yes, I think that makes sense, i.e. '--submodules' will include _all_
> submodules (making the option behave identically for bare and non-bare
> repositories), '--submodules=.' will include checked out submodules
> (making the option a no-op in bare repos, which also makes sense) and
> '--submodules=<name>[,<name>...]' will include the named submodules,
> where "named" could mean groupname, submodule name or submodule path, in
> that order.
Well, I can live with the default of all submodules, even if I think that
"git-submodule.sh" uses the checked out submodules by default.
> But then we probably also want some (optional) syntax to specify the
> kind of name, e.g. '--submodules=g:foo,n:bar,p:lib/baz' for group foo,
> name bar and path lib/baz. Agree?
IMO that is overkill. Anybody naming a submodule group identically to a
submodule deserves what she gets, anyway.
> >> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)
> >> check[1].attr = attr_export_subst;
> >> }
> >>
> >> +static int include_repository(const char *path)
> >> +{
> >> + struct stat st;
> >> + const char *tmp;
> >> +
> >> + /* Return early if the path does not exist since it is OK to not
> >> + * checkout submodules.
> >> + */
> >> + if (stat(path, &st) && errno == ENOENT)
> >> + return 1;
> >> +
> >> + tmp = read_gitfile_gently(path);
> >
> > This will leak memory, no?
>
> I don't think so: read_gitfile_gently() returns a value obtained by
> calling make_absolute_path() which returns a static buffer. Also, the
> path argument to include_repository() is obtained by calling mkpath()
> which returns another static buffer so I don't see any malloc()'s
> which should be free()'d. Is my code-reading flawed?
No, your code reading is good. And you spared me having to read the code
myself ;-) Now, maybe a code comment is in order, to spare others, too?
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
