Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()

[Max Kirillov]

On Fri, Sep 11, 2015 at 03:53:40AM -0400, Jeff King wrote:
> 
> 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(_directory, "%s/.git", path);
> - git_dir = read_gitfile(objects_directory.buf);
> - if (git_dir) {
> - strbuf_reset(_directory);
> - strbuf_addstr(_directory, git_dir);
> - }
> - strbuf_addstr(_directory, "/objects/");
> + strbuf_git_path_submodule(_directory, path, "objects/");
>   if (!is_directory(objects_directory.buf)) {
>   ret = -1;
>   goto done;
> 
> -Peff

Thank you. This is how it looks now.

-- 
Max
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()

[Jeff King]

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(_directory, "%s/.git", path);
-   git_dir = read_gitfile(objects_directory.buf);
-   if (git_dir) {
-   strbuf_reset(_directory);
-   strbuf_addstr(_directory, git_dir);
-   }
-   strbuf_addstr(_directory, "/objects/");
+   strbuf_git_path_submodule(_directory, path, "objects/");
if (!is_directory(objects_directory.buf)) {
ret = -1;
goto done;

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html