From: Jeff King <peff@peff.net>
Date: Fri, 11 Sep 2015 07:53:40 GMT
Subject: Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()
Message-ID: <20150911075340.GA18917@sigill.intra.peff.net>
In-Reply-To: <1441922231-18270-2-git-send-email-max@max630.net>

On Fri, Sep 11, 2015 at 12:57:10AM +0300, Max Kirillov wrote:

> diff --git a/submodule.c b/submodule.c
> index 245ed4d..7340069 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -119,43 +119,37 @@ void stage_updated_gitmodules(void)
>  
>  static int add_submodule_odb(const char *path)
>  {
> -	struct strbuf objects_directory = STRBUF_INIT;
>  	struct alternate_object_database *alt_odb;
> +	char *objects_directory;

Now that we have strbuf_git_path_submodule(), is there any reason to
switch this away from a strbuf?

That saves us a bunch of strlen calls, and it makes the diff way
shorter. My ulterior motive is that the result also conflicts a lot less
with some patches I'm about to post to harden the malloc and strcpy
calls below.

That would make your patch look something like:

diff --git a/submodule.c b/submodule.c
index 245ed4d..5e5a46f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path)
 	struct strbuf objects_directory = STRBUF_INIT;
 	struct alternate_object_database *alt_odb;
 	int ret = 0;
-	const char *git_dir;
 
-	strbuf_addf(&objects_directory, "%s/.git", path);
-	git_dir = read_gitfile(objects_directory.buf);
-	if (git_dir) {
-		strbuf_reset(&objects_directory);
-		strbuf_addstr(&objects_directory, git_dir);
-	}
-	strbuf_addstr(&objects_directory, "/objects/");
+	strbuf_git_path_submodule(&objects_directory, path, "objects/");
 	if (!is_directory(objects_directory.buf)) {
 		ret = -1;
 		goto done;

-Peff