Re: [PATCH v4 02/27] Convert git_snpath() to strbuf_git_path()

[Eric Sunshine <sunshine@sunshineco.com>]

On Sat, Mar 1, 2014 at 7:12 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> In the previous patch, git_snpath() is modified to allocate a new
> strbuf buffer because vsnpath() needs that. But that makes it awkward
> because git_snpath() receives a pre-allocated buffer from outside and
> has to copy data back. Rename it to strbuf_git_path() and make it
> receive strbuf directly.
>
> The conversion from git_snpath() to git_path() in
> update_refs_for_switch() is safe because that function does not keep
> any pointer to the round-robin buffer pool allocated by
> get_pathname().
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/refs.c b/refs.c
> index 89228e2..434bd5e 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2717,17 +2729,19 @@ static int copy_msg(char *buf, const char *msg)
>         return cp - buf;
>  }
>
> -int log_ref_setup(const char *refname, char *logfile, int bufsize)
> +int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
>  {
>         int logfd, oflags = O_APPEND | O_WRONLY;
> +       const char *logfile;
>
> -       git_snpath(logfile, bufsize, "logs/%s", refname);
> +       strbuf_git_path(sb_logfile, "logs/%s", refname);
> +       logfile = sb_logfile->buf;
>         if (log_all_ref_updates &&
>             (starts_with(refname, "refs/heads/") ||
>              starts_with(refname, "refs/remotes/") ||
>              starts_with(refname, "refs/notes/") ||
>              !strcmp(refname, "HEAD"))) {
> -               if (safe_create_leading_directories(logfile) < 0)
> +               if (safe_create_leading_directories(sb_logfile->buf) < 0)

At this point, 'logfile' is still 'sb_logfile->buf', so do you really
need this change?

>                         return error("unable to create directory for %s",
>                                      logfile);
>                 oflags |= O_CREAT;
> @@ -2762,20 +2776,22 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
>         int logfd, result, written, oflags = O_APPEND | O_WRONLY;
>         unsigned maxlen, len;
>         int msglen;
> -       char log_file[PATH_MAX];
> +       struct strbuf sb_log_file = STRBUF_INIT;
> +       const char *log_file;
>         char *logrec;
>         const char *committer;
>
>         if (log_all_ref_updates < 0)
>                 log_all_ref_updates = !is_bare_repository();
>
> -       result = log_ref_setup(refname, log_file, sizeof(log_file));
> +       result = log_ref_setup(refname, &sb_log_file);
>         if (result)
> -               return result;
> +               goto done;
> +       log_file = sb_log_file.buf;
>
>         logfd = open(log_file, oflags);
>         if (logfd < 0)
> -               return 0;
> +               goto done;
>         msglen = msg ? strlen(msg) : 0;
>         committer = git_committer_info(0);
>         maxlen = strlen(committer) + msglen + 100;
> --
> 1.9.0.40.gaa8c3ea