Re: [PATCH 10/17] path.c: drop git_path_submodule
On Mon, Aug 10, 2015 at 03:57:31PM -0700, Junio C Hamano wrote:
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
There are a few callers added on 'pu', though.
Actually there is only one. Here is a proposed evil merge.
Thanks for catching. I (obviously) only checked against 'next' and not
'pu'. Rather than the evil merge, we could also just drop this patch.
And then either leave it be, or fix this one up as a separate topic once
merged. Though it really looks like this call site is potentially
dangerous, with the assignment (I think it is OK, though, because
read_info_alternates does not use git_path itself).
diff --git a/submodule.c b/submodule.c
index dfe8b7b..7ab08a1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -120,34 +120,35 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
[...]
{
struct alternate_object_database *alt_odb;
- const char *objects_directory;
+ struct strbuf objects_directory = STRBUF_INIT;
int ret = 0;
- objects_directory = git_path_submodule(path, objects/);
- if (!is_directory(objects_directory)) {
+ strbuf_git_path_submodule(objects_directory, objects/, %s, path);
+ if (!is_directory(objects_directory.buf)) {
ret = -1;
goto done;
}
Hmph, the change in 6659e74 would have been a lot nicer if
strbuf_git_path_submodule were already available. Most of what you are
doing here is undoing that commit's strbuf/buf transition. :-/
/* avoid adding it twice */
for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb-next)
- if (alt_odb-name - alt_odb-base == strlen(objects_directory)
- !strcmp(alt_odb-base, objects_directory))
+ if (alt_odb-name - alt_odb-base == objects_directory.len
+ !strcmp(alt_odb-base, objects_directory.buf))
goto done;
Not really relevant to what we're talking about here, but this is the
first time I've lookd at add_submodule_odb. It really looks like the
whole second half of the function could be replaced with a call to
link_alt_odb_entry.
-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
Re: [PATCH 10/17] path.c: drop git_path_submodule
Jeff King p...@peff.net writes: There are no callers of the slightly-dangerous static-buffer git_path_submodule left. Let's drop it. There are a few callers added on 'pu', though. -- 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 10/17] path.c: drop git_path_submodule
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
There are a few callers added on 'pu', though.
Actually there is only one. Here is a proposed evil merge.
diff --git a/submodule.c b/submodule.c
index dfe8b7b..7ab08a1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -120,34 +120,35 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
{
struct alternate_object_database *alt_odb;
- const char *objects_directory;
+ struct strbuf objects_directory = STRBUF_INIT;
int ret = 0;
- objects_directory = git_path_submodule(path, objects/);
- if (!is_directory(objects_directory)) {
+ strbuf_git_path_submodule(objects_directory, objects/, %s, path);
+ if (!is_directory(objects_directory.buf)) {
ret = -1;
goto done;
}
/* avoid adding it twice */
for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb-next)
- if (alt_odb-name - alt_odb-base == strlen(objects_directory)
- !strcmp(alt_odb-base, objects_directory))
+ if (alt_odb-name - alt_odb-base == objects_directory.len
+ !strcmp(alt_odb-base, objects_directory.buf))
goto done;
- alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));
+ alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
alt_odb-next = alt_odb_list;
- strcpy(alt_odb-base, objects_directory);
- alt_odb-name = alt_odb-base + strlen(objects_directory);
+ strcpy(alt_odb-base, objects_directory.buf);
+ alt_odb-name = alt_odb-base + objects_directory.len;
alt_odb-name[2] = '/';
alt_odb-name[40] = '\0';
alt_odb-name[41] = '\0';
alt_odb_list = alt_odb;
/* add possible alternates from the submodule */
- read_info_alternates(objects_directory, 0);
+ read_info_alternates(objects_directory.buf, 0);
prepare_alt_odb();
done:
+ strbuf_release(objects_directory);
return ret;
}
--
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 10/17] path.c: drop git_path_submodule
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
There are a few callers added on 'pu', though.
Actually there is only one. Here is a proposed evil merge.
Sorry, that didn't work X-.
diff --git a/submodule.c b/submodule.c
index dfe8b7b..0cdaeb8 100644
--- a/submodule.c
+++ b/submodule.c
@@ -120,10 +120,10 @@ void stage_updated_gitmodules(void)
static int add_submodule_odb(const char *path)
{
struct alternate_object_database *alt_odb;
- const char *objects_directory;
+ char *objects_directory;
int ret = 0;
- objects_directory = git_path_submodule(path, objects/);
+ objects_directory = git_pathdup_submodule(path, objects/);
if (!is_directory(objects_directory)) {
ret = -1;
goto done;
@@ -148,6 +148,7 @@ static int add_submodule_odb(const char *path)
read_info_alternates(objects_directory, 0);
prepare_alt_odb();
done:
+ free(objects_directory);
return ret;
}
--
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
[PATCH 10/17] path.c: drop git_path_submodule
There are no callers of the slightly-dangerous static-buffer
git_path_submodule left. Let's drop it.
Signed-off-by: Jeff King p...@peff.net
---
cache.h | 5 ++---
path.c | 10 --
2 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/cache.h b/cache.h
index 6f74f33..7a4fa90 100644
--- a/cache.h
+++ b/cache.h
@@ -713,12 +713,11 @@ extern int check_repository_format(void);
* the repository directory (git_path), or in a submodule's repository
* directory (git_path_submodule). In all cases, note that the result
* may be overwritten by another call to _any_ of the functions. Consider
- * using the safer dup or strbuf formats below.
+ * using the safer dup or strbuf formats below (in some cases, the
+ * unsafe versions have already been removed).
*/
extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf,
1, 2)));
extern const char *git_path(const char *fmt, ...) __attribute__((format
(printf, 1, 2)));
-extern const char *git_path_submodule(const char *path, const char *fmt, ...)
- __attribute__((format (printf, 2, 3)));
extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
diff --git a/path.c b/path.c
index 9aad9a1..94d7ec2 100644
--- a/path.c
+++ b/path.c
@@ -245,16 +245,6 @@ static void do_submodule_path(struct strbuf *buf, const
char *path,
strbuf_cleanup_path(buf);
}
-const char *git_path_submodule(const char *path, const char *fmt, ...)
-{
- va_list args;
- struct strbuf *buf = get_pathname();
- va_start(args, fmt);
- do_submodule_path(buf, path, fmt, args);
- va_end(args);
- return buf-buf;
-}
-
char *git_pathdup_submodule(const char *path, const char *fmt, ...)
{
va_list args;
--
2.5.0.414.g670f2a4
--
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
