From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH 0/17] removing questionable uses of git_path
Date: Mon, 10 Aug 2015 10:31:32 -0700 [thread overview]
Message-ID: <xmqqr3nbuk23.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150810092731.GA9027@sigill.intra.peff.net> (Jeff King's message of "Mon, 10 Aug 2015 05:27:32 -0400")
Jeff King <peff@peff.net> writes:
> The problem is that git_path uses a static buffer that gets overwritten
> by subsequent calls.
As the rotating static buffer pattern used in get_pathname() was
modeled after sha1_to_hex(), we have the same issue there. They are
troubles waiting to happen unless the callers are careful.
> producing a fairly tame-looking set of function calls. It's OK to pass
> the result of git_path() to a system call, or something that is a thin
> wrapper around one (e.g., strbuf_read_file).
That is a short and good rule to follow, but the problem is that not
everybody has good taste to interpret the above rule and apply it with
an eye toward maintainability X-<.
> Along the way, there are a few cleanups (e.g., I polished off the recent
> hold_lock_file_for_append topic which was on the list, as it had some
> problematic calls).
>
> [01/17]: cache.h: clarify documentation for git_path, et al
> [02/17]: cache.h: complete set of git_path_submodule helpers
> [03/17]: t5700: modernize style
> [04/17]: add_to_alternates_file: don't add duplicate entries
> [05/17]: remove hold_lock_file_for_append
> [06/17]: prefer git_pathdup to git_path in some possibly-dangerous cases
> [07/17]: prefer mkpathdup to mkpath in assignments
> [08/17]: remote.c: drop extraneous local variable from migrate_file
> [09/17]: refs.c: remove extra git_path calls from read_loose refs
> [10/17]: path.c: drop git_path_submodule
> [11/17]: refs.c: simplify strbufs in reflog setup and writing
> [12/17]: refs.c: avoid repeated git_path calls in rename_tmp_log
> [13/17]: refs.c: avoid git_path assignment in lock_ref_sha1_basic
> [14/17]: refs.c: remove_empty_directories can take a strbuf
> [15/17]: find_hook: keep our own static buffer
> [16/17]: get_repo_path: refactor path-allocation
> [17/17]: memoize common git-path "constant" files
Nice. Thanks.
next prev parent reply other threads:[~2015-08-10 17:31 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-10 9:27 [PATCH 0/17] removing questionable uses of git_path Jeff King
2015-08-10 9:32 ` [PATCH 01/17] cache.h: clarify documentation for git_path, et al Jeff King
2015-08-10 9:32 ` [PATCH 02/17] cache.h: complete set of git_path_submodule helpers Jeff King
2015-08-10 9:32 ` [PATCH 03/17] t5700: modernize style Jeff King
2015-08-10 9:34 ` [PATCH 04/17] add_to_alternates_file: don't add duplicate entries Jeff King
2015-08-11 4:00 ` Michael Haggerty
2015-08-11 9:54 ` Jeff King
2015-08-10 9:35 ` [PATCH 05/17] remove hold_lock_file_for_append Jeff King
2015-08-10 22:36 ` Junio C Hamano
2015-08-11 9:38 ` Jeff King
2015-08-10 9:35 ` [PATCH 06/17] prefer git_pathdup to git_path in some possibly-dangerous cases Jeff King
2015-08-10 9:35 ` [PATCH 07/17] prefer mkpathdup to mkpath in assignments Jeff King
2015-08-10 9:35 ` [PATCH 08/17] remote.c: drop extraneous local variable from migrate_file Jeff King
2015-08-10 9:36 ` [PATCH 09/17] refs.c: remove extra git_path calls from read_loose_refs Jeff King
2015-08-10 9:36 ` [PATCH 10/17] path.c: drop git_path_submodule Jeff King
2015-08-10 22:50 ` Junio C Hamano
2015-08-10 22:57 ` Junio C Hamano
2015-08-10 23:52 ` Junio C Hamano
2015-08-11 9:53 ` Jeff King
2015-08-10 9:36 ` [PATCH 11/17] refs.c: simplify strbufs in reflog setup and writing Jeff King
2015-08-10 10:34 ` Michael Haggerty
2015-08-10 12:26 ` Jeff King
2015-08-10 9:36 ` [PATCH 12/17] refs.c: avoid repeated git_path calls in rename_tmp_log Jeff King
2015-08-10 9:37 ` [PATCH 13/17] refs.c: avoid git_path assignment in lock_ref_sha1_basic Jeff King
2015-08-10 9:37 ` [PATCH 14/17] refs.c: remove_empty_directories can take a strbuf Jeff King
2015-08-10 9:37 ` [PATCH 15/17] find_hook: keep our own static buffer Jeff King
2015-08-10 9:37 ` [PATCH 16/17] get_repo_path: refactor path-allocation Jeff King
2015-08-10 9:38 ` [PATCH 17/17] memoize common git-path "constant" files Jeff King
2015-08-10 12:05 ` Michael Haggerty
2015-08-10 12:30 ` Jeff King
2015-08-10 12:06 ` [PATCH 0/17] removing questionable uses of git_path Michael Haggerty
2015-08-10 17:31 ` Junio C Hamano [this message]
2015-08-10 17:47 ` Jeff King
2015-08-15 9:05 ` Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqr3nbuk23.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mhagger@alum.mit.edu \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.