Re: [PATCH v2] builtin/commit.c: memoize git-path for COMMIT_EDITMSG

[Pranit Bauva <pranit.bauva@gmail.com>]

On Wed, May 25, 2016 at 12:49 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> This is a follow up commit for f932729c (memoize common git-path
> "constant" files, 10-Aug-2015).
>
> The many function calls to git_path() are replaced by
> git_path_commit_editmsg() and which thus eliminates the need to repeatedly
> compute the location of "COMMIT_EDITMSG".
>
> Mentored-by: Lars Schneider <larsxschneider@gmail.com>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> ---
> Link for v1[1].
>
> Changes wrt v1:
>
>  * Remove the call to git_path_commit_editmsg() which would directly assign
>    the value to the string.
>  * Remove the string commit_editmsg[] as it is redundant now.
>  * Call git_path_commit_editmsg() everytime when it is needed.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/295345
>
>  builtin/commit.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 391126e..01b921f 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -92,8 +92,9 @@ N_("If you wish to skip this commit, use:\n"
>  "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
>  "the remaining commits.\n");
>
> +static GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
> +
>  static const char *use_message_buffer;
> -static const char commit_editmsg[] = "COMMIT_EDITMSG";
>  static struct lock_file index_lock; /* real index */
>  static struct lock_file false_lock; /* used only for partial commits */
>  static enum {
> @@ -771,9 +772,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>                 hook_arg2 = "";
>         }
>
> -       s->fp = fopen_for_writing(git_path(commit_editmsg));
> +       s->fp = fopen_for_writing(git_path_commit_editmsg());
>         if (s->fp == NULL)
> -               die_errno(_("could not open '%s'"), git_path(commit_editmsg));
> +               die_errno(_("could not open '%s'"), git_path_commit_editmsg());
>
>         /* Ignore status.displayCommentPrefix: we do need comments in COMMIT_EDITMSG. */
>         old_display_comment_prefix = s->display_comment_prefix;
> @@ -950,7 +951,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>         }
>
>         if (run_commit_hook(use_editor, index_file, "prepare-commit-msg",
> -                           git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
> +                           git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL))
>                 return 0;
>
>         if (use_editor) {
> @@ -958,7 +959,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>                 const char *env[2] = { NULL };
>                 env[0] =  index;
>                 snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> -               if (launch_editor(git_path(commit_editmsg), NULL, env)) {
> +               if (launch_editor(git_path_commit_editmsg(), NULL, env)) {
>                         fprintf(stderr,
>                         _("Please supply the message using either -m or -F option.\n"));
>                         exit(1);
> @@ -966,7 +967,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>         }
>
>         if (!no_verify &&
> -           run_commit_hook(use_editor, index_file, "commit-msg", git_path(commit_editmsg), NULL)) {
> +           run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) {
>                 return 0;
>         }
>
> @@ -1728,7 +1729,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>
>         /* Finally, get the commit message */
>         strbuf_reset(&sb);
> -       if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
> +       if (strbuf_read_file(&sb, git_path_commit_editmsg(), 0) < 0) {
>                 int saved_errno = errno;
>                 rollback_index_files();
>                 die(_("could not read commit message: %s"), strerror(saved_errno));
> --
> 2.8.3
>

Anyone any comments?

Regards,
Pranit Bauva