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 a "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 ad74624c2f7..891572d807c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -116,6 +116,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);
@@ -157,10 +161,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;
}
@@ -174,11 +176,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);
@@ -197,7 +196,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);
@@ -598,10 +597,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,
@@ -613,9 +612,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;
}
@@ -662,9 +662,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()
@@ -674,21 +674,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,
@@ -700,7 +701,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)))
@@ -709,15 +710,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()
@@ -734,7 +736,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);
@@ -746,8 +748,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;
}
@@ -825,8 +827,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;
@@ -861,10 +863,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
@@ -878,8 +878,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;
}
}
@@ -912,8 +912,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,
@@ -1409,9 +1412,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" */
@@ -1426,6 +1429,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;
}
@@ -1434,7 +1438,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.1557.g4e3e9cdb5e0
