Change the initialization of the "revision" member of "struct
stash_info" to be initialized vi a macro, and more importantly that
that initializing function be tasked to free it, usually via "goto
cleanup" pattern.
Despite the "revision" name (and the topic of the series containing
this commit) the "stash info" has nothing to do with the "struct
rev_info". I'm making this change because in the subsequent commit
when we do want to free the "struct rev_info" via a "goto cleanup"
pattern we'd otherwise free() uninitialized memory in some cases, as
we only strbuf_init() the string in get_stash_info().
So while it's the smallest possible change, let's convert all users of
this pattern in the file while we're at it.
A good follow-up to this change would be to change all the "ret = -1;
goto done;" in this file to instead use a "goto cleanup", and
initialize "int ret = -1" at the start of the relevant functions. That
would allow us to drop a lot of needless brace verbosity on two-line
"if" statements, but let's leave that alone for now.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
builtin/stash.c | 70 ++++++++++++++++++++++++++-----------------------
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index 8a8093c5eea..4c8c8af8a09 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -117,6 +117,10 @@ struct stash_info {
int has_u;
};
+#define STASH_INFO_INIT { \
+ .revision = STRBUF_INIT, \
+}
+
static void free_stash_info(struct stash_info *info)
{
strbuf_release(&info->revision);
@@ -158,10 +162,8 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
if (argc == 1)
commit = argv[0];
- strbuf_init(&info->revision, 0);
if (!commit) {
if (!ref_exists(ref_stash)) {
- free_stash_info(info);
fprintf_ln(stderr, _("No stash entries found."));
return -1;
}
@@ -175,11 +177,8 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
revision = info->revision.buf;
- if (get_oid(revision, &info->w_commit)) {
- error(_("%s is not a valid reference"), revision);
- free_stash_info(info);
- return -1;
- }
+ if (get_oid(revision, &info->w_commit))
+ return error(_("%s is not a valid reference"), revision);
assert_stash_like(info, revision);
@@ -198,7 +197,7 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)
info->is_stash_ref = !strcmp(expanded_ref, ref_stash);
break;
default: /* Invalid or ambiguous */
- free_stash_info(info);
+ break;
}
free(expanded_ref);
@@ -599,10 +598,10 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
static int apply_stash(int argc, const char **argv, const char *prefix)
{
- int ret;
+ int ret = -1;
int quiet = 0;
int index = 0;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_BOOL(0, "index", &index,
@@ -614,9 +613,10 @@ static int apply_stash(int argc, const char **argv, const char *prefix)
git_stash_apply_usage, 0);
if (get_stash_info(&info, argc, argv))
- return -1;
+ goto cleanup;
ret = do_apply_stash(prefix, &info, index, quiet);
+cleanup:
free_stash_info(&info);
return ret;
}
@@ -663,9 +663,9 @@ static void assert_stash_ref(struct stash_info *info)
static int drop_stash(int argc, const char **argv, const char *prefix)
{
- int ret;
+ int ret = -1;
int quiet = 0;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_END()
@@ -675,21 +675,22 @@ static int drop_stash(int argc, const char **argv, const char *prefix)
git_stash_drop_usage, 0);
if (get_stash_info(&info, argc, argv))
- return -1;
+ goto cleanup;
assert_stash_ref(&info);
ret = do_drop_stash(&info, quiet);
+cleanup:
free_stash_info(&info);
return ret;
}
static int pop_stash(int argc, const char **argv, const char *prefix)
{
- int ret;
+ int ret = -1;
int index = 0;
int quiet = 0;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_BOOL(0, "index", &index,
@@ -701,7 +702,7 @@ static int pop_stash(int argc, const char **argv, const char *prefix)
git_stash_pop_usage, 0);
if (get_stash_info(&info, argc, argv))
- return -1;
+ goto cleanup;
assert_stash_ref(&info);
if ((ret = do_apply_stash(prefix, &info, index, quiet)))
@@ -710,15 +711,16 @@ static int pop_stash(int argc, const char **argv, const char *prefix)
else
ret = do_drop_stash(&info, quiet);
+cleanup:
free_stash_info(&info);
return ret;
}
static int branch_stash(int argc, const char **argv, const char *prefix)
{
- int ret;
+ int ret = -1;
const char *branch = NULL;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
struct option options[] = {
OPT_END()
@@ -735,7 +737,7 @@ static int branch_stash(int argc, const char **argv, const char *prefix)
branch = argv[0];
if (get_stash_info(&info, argc - 1, argv + 1))
- return -1;
+ goto cleanup;
cp.git_cmd = 1;
strvec_pushl(&cp.args, "checkout", "-b", NULL);
@@ -747,8 +749,8 @@ static int branch_stash(int argc, const char **argv, const char *prefix)
if (!ret && info.is_stash_ref)
ret = do_drop_stash(&info, 0);
+cleanup:
free_stash_info(&info);
-
return ret;
}
@@ -826,8 +828,8 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
static int show_stash(int argc, const char **argv, const char *prefix)
{
int i;
- int ret = 0;
- struct stash_info info;
+ int ret = -1;
+ struct stash_info info = STASH_INFO_INIT;
struct rev_info rev;
struct strvec stash_args = STRVEC_INIT;
struct strvec revision_args = STRVEC_INIT;
@@ -862,10 +864,8 @@ static int show_stash(int argc, const char **argv, const char *prefix)
strvec_push(&revision_args, argv[i]);
}
- ret = get_stash_info(&info, stash_args.nr, stash_args.v);
- strvec_clear(&stash_args);
- if (ret)
- return -1;
+ if (get_stash_info(&info, stash_args.nr, stash_args.v))
+ goto cleanup;
/*
* The config settings are applied only if there are not passed
@@ -879,8 +879,8 @@ static int show_stash(int argc, const char **argv, const char *prefix)
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
if (!show_stat && !show_patch) {
- free_stash_info(&info);
- return 0;
+ ret = 0;
+ goto cleanup;
}
}
@@ -913,8 +913,11 @@ static int show_stash(int argc, const char **argv, const char *prefix)
}
log_tree_diff_flush(&rev);
+ ret = diff_result_code(&rev.diffopt, 0);;
+cleanup:
+ strvec_clear(&stash_args);
free_stash_info(&info);
- return diff_result_code(&rev.diffopt, 0);
+ return ret;
}
static int do_store_stash(const struct object_id *w_commit, const char *stash_msg,
@@ -1410,9 +1413,9 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
static int create_stash(int argc, const char **argv, const char *prefix)
{
- int ret = 0;
+ int ret;
struct strbuf stash_msg_buf = STRBUF_INIT;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct pathspec ps;
/* Starting with argv[1], since argv[0] is "create" */
@@ -1427,6 +1430,7 @@ static int create_stash(int argc, const char **argv, const char *prefix)
if (!ret)
printf_ln("%s", oid_to_hex(&info.w_commit));
+ free_stash_info(&info);
strbuf_release(&stash_msg_buf);
return ret;
}
@@ -1435,7 +1439,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
int keep_index, int patch_mode, int include_untracked, int only_staged)
{
int ret = 0;
- struct stash_info info;
+ struct stash_info info = STASH_INFO_INIT;
struct strbuf patch = STRBUF_INIT;
struct strbuf stash_msg_buf = STRBUF_INIT;
struct strbuf untracked_files = STRBUF_INIT;
--
2.35.1.1452.ga7cfc89151f
