On Wed, Nov 16, 2011 at 03:50:04PM -0600, Jonathan Nieder wrote:
> strbuf_addpath() is like git_path_unsafe(), except instead of
> returning its own buffer, it appends its result to a buffer provided
> by the caller.
>
> Benefits:
>
> - Since it uses a caller-supplied buffer, unlike git_path_unsafe(),
> there is no risk that one call will clobber the result from
> another.
>
> - Unlike git_pathdup(), it does not need to waste time allocating
> memory in the middle of your tight loop over refs.
>
> - The size of the result is not limited to PATH_MAX.
>
> Caveat: the size of its result is not limited to PATH_MAX. Existing
> code might be relying on git_path*() to produce a result that is safe
> to copy to a PATH_MAX-sized buffer. Be careful.
>
> This patch introduces the strbuf_addpath() function and converts a few
> existing users of the strbuf_addstr(git_path(...)) idiom to
> demonstrate the API.
>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> Jonathan Nieder wrote:
>
> > I think if I ran the world, the fundamental operation would be
> > strbuf_addpath().
>
> Like this, maybe.
>
If I ran the world, I would change get_pathname() to return "struct
strbuf *" instead and change "char *git_path(..)" to "const char *git_path(...)".
Code paths that want to modify git_path() return value could
just use strbuf_addpath()
I did try to turn git_path to return const char *, the following patch
seems to make it build without any warnings
-- 8< --
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb..fd7c682 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -1041,7 +1041,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
if (args.depth > 0) {
struct cache_time mtime;
struct strbuf sb = STRBUF_INIT;
- char *shallow = git_path("shallow");
+ const char *shallow = git_path("shallow");
int fd;
mtime.sec = st.st_mtime;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 91731b9..261f1a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -367,7 +367,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
char note[1024];
const char *what, *kind;
struct ref *rm;
- char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ const char *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
+ char *url;
fp = fopen(filename, "a");
if (!fp)
@@ -647,7 +648,7 @@ static void check_not_current_branch(struct ref *ref_map)
static int truncate_fetch_head(void)
{
- char *filename = git_path("FETCH_HEAD");
+ const char *filename = git_path("FETCH_HEAD");
FILE *fp = fopen(filename, "w");
if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0e0e17a..f9624d7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -215,12 +215,12 @@ static void check_unreachable_object(struct object *obj)
printf("dangling %s %s\n", typename(obj->type),
sha1_to_hex(obj->sha1));
if (write_lost_and_found) {
- char *filename = git_path("lost-found/%s/%s",
+ const char *filename = git_path("lost-found/%s/%s",
obj->type == OBJ_COMMIT ? "commit" : "other",
sha1_to_hex(obj->sha1));
FILE *f;
- if (safe_create_leading_directories(filename)) {
+ if (safe_create_leading_directories_const(filename)) {
error("Could not create lost-found");
return;
}
diff --git a/builtin/remote.c b/builtin/remote.c
index 583eec9..0662d37 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -587,7 +587,7 @@ static int migrate_file(struct remote *remote)
{
struct strbuf buf = STRBUF_INIT;
int i;
- char *path = NULL;
+ const char *path = NULL;
strbuf_addf(&buf, "remote.%s.url", remote->name);
for (i = 0; i < remote->url_nr; i++)
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..4293a9f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -403,7 +403,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);
static void write_crash_report(const char *err)
{
- char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
+ const char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());
FILE *rpt = fopen(loc, "w");
struct branch *b;
unsigned long lu;
diff --git a/notes-merge.c b/notes-merge.c
index e33c2c9..8da2e0a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -288,7 +288,7 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
"(%s exists).", git_path("NOTES_MERGE_*"));
}
- if (safe_create_leading_directories(git_path(
+ if (safe_create_leading_directories_const(git_path(
NOTES_MERGE_WORKTREE "/.test")))
die_errno("unable to create directory %s",
git_path(NOTES_MERGE_WORKTREE));
@@ -303,8 +303,8 @@ static void write_buf_to_worktree(const unsigned char *obj,
const char *buf, unsigned long size)
{
int fd;
- char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
- if (safe_create_leading_directories(path))
+ const char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
+ if (safe_create_leading_directories_const(path))
die_errno("unable to create directory for '%s'", path);
if (file_exists(path))
die("found existing file at '%s'", path);
diff --git a/refs.c b/refs.c
index 62d8a37..7469cf1 100644
--- a/refs.c
+++ b/refs.c
@@ -1189,7 +1189,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
{
- char *ref_file;
+ const char *ref_file;
const char *orig_ref = ref;
struct ref_lock *lock;
int last_errno = 0;
@@ -1250,7 +1250,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
if ((flags & REF_NODEREF) && (type & REF_ISSYMREF))
lock->force_write = 1;
- if (safe_create_leading_directories(ref_file)) {
+ if (safe_create_leading_directories_const(ref_file)) {
last_errno = errno;
error("unable to create directory for %s", ref_file);
goto error_return;
@@ -1411,7 +1411,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
}
}
- if (log && safe_create_leading_directories(git_path("logs/%s", newref))) {
+ if (log && safe_create_leading_directories_const(git_path("logs/%s", newref))) {
error("unable to create directory for %s", newref);
goto rollback;
}
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html