sequencer: fix leaking string buffer in `commit_staged_changes()`
We're leaking the `rev` string buffer in various call paths. Refactor the function to have a common exit path so that we can release its memory reliably. This fixes a subset of tests failing with the memory sanitizer in t3404. But as there are more failures, we cannot yet mark the whole test suite as passing. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jun 11, 2024 at 11:20 UTC
1e5c1601f98afba0772c4548ec6befe6e97761e7
1 file changed
+73
-38
sequencer.c
+73
-38
@@ -5146,33 +5146,47 @@ static int commit_staged_changes(struct repository *r,
5146
struct replay_ctx *ctx = opts->ctx;
5147
unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
5148
unsigned int final_fixup = 0, is_clean;
5149
+ struct strbuf rev = STRBUF_INIT;
5150
+ int ret;
5151
5150
- if (has_unstaged_changes(r, 1))
5151
- return error(_("cannot rebase: You have unstaged changes."));
5152
+ if (has_unstaged_changes(r, 1)) {
5153
+ ret = error(_("cannot rebase: You have unstaged changes."));
5154
+ goto out;
5155
+ }
5156
5157
is_clean = !has_uncommitted_changes(r, 0);
5158
5159
if (!is_clean && !file_exists(rebase_path_message())) {
5160
const char *gpg_opt = gpg_sign_opt_quoted(opts);
5157
-
5158
- return error(_(staged_changes_advice), gpg_opt, gpg_opt);
5161
+ ret = error(_(staged_changes_advice), gpg_opt, gpg_opt);
5162
+ goto out;
5163
}
5164
+
5165
if (file_exists(rebase_path_amend())) {
5161
- struct strbuf rev = STRBUF_INIT;
5166
struct object_id head, to_amend;
5167
5164
- if (repo_get_oid(r, "HEAD", &head))
5165
- return error(_("cannot amend non-existing commit"));
5166
- if (!read_oneliner(&rev, rebase_path_amend(), 0))
5167
- return error(_("invalid file: '%s'"), rebase_path_amend());
5168
- if (get_oid_hex(rev.buf, &to_amend))
5169
- return error(_("invalid contents: '%s'"),
5170
- rebase_path_amend());
5171
- if (!is_clean && !oideq(&head, &to_amend))
5172
- return error(_("\nYou have uncommitted changes in your "
5173
- "working tree. Please, commit them\n"
5174
- "first and then run 'git rebase "
5175
- "--continue' again."));
5168
+ if (repo_get_oid(r, "HEAD", &head)) {
5169
+ ret = error(_("cannot amend non-existing commit"));
5170
+ goto out;
5171
+ }
5172
+
5173
+ if (!read_oneliner(&rev, rebase_path_amend(), 0)) {
5174
+ ret = error(_("invalid file: '%s'"), rebase_path_amend());
5175
+ goto out;
5176
+ }
5177
+
5178
+ if (get_oid_hex(rev.buf, &to_amend)) {
5179
+ ret = error(_("invalid contents: '%s'"),
5180
+ rebase_path_amend());
5181
+ goto out;
5182
+ }
5183
+ if (!is_clean && !oideq(&head, &to_amend)) {
5184
+ ret = error(_("\nYou have uncommitted changes in your "
5185
+ "working tree. Please, commit them\n"
5186
+ "first and then run 'git rebase "
5187
+ "--continue' again."));
5188
+ goto out;
5189
+ }
5190
/*
5191
* When skipping a failed fixup/squash, we need to edit the
5192
* commit message, the current fixup list and count, and if it
@@ -5204,9 +5218,11 @@ static int commit_staged_changes(struct repository *r,
5218
len--;
5219
strbuf_setlen(&ctx->current_fixups, len);
5220
if (write_message(p, len, rebase_path_current_fixups(),
5207
- 0) < 0)
5208
- return error(_("could not write file: '%s'"),
5209
- rebase_path_current_fixups());
5221
+ 0) < 0) {
5222
+ ret = error(_("could not write file: '%s'"),
5223
+ rebase_path_current_fixups());
5224
+ goto out;
5225
+ }
5226
5227
/*
5228
* If a fixup/squash in a fixup/squash chain failed, the
@@ -5236,35 +5252,38 @@ static int commit_staged_changes(struct repository *r,
5252
* We need to update the squash message to skip
5253
* the latest commit message.
5254
*/
5239
- int res = 0;
5255
struct commit *commit;
5256
const char *msg;
5257
const char *path = rebase_path_squash_msg();
5258
const char *encoding = get_commit_output_encoding();
5259
5245
- if (parse_head(r, &commit))
5246
- return error(_("could not parse HEAD"));
5260
+ if (parse_head(r, &commit)) {
5261
+ ret = error(_("could not parse HEAD"));
5262
+ goto out;
5263
+ }
5264
5265
p = repo_logmsg_reencode(r, commit, NULL, encoding);
5266
if (!p) {
5250
- res = error(_("could not parse commit %s"),
5267
+ ret = error(_("could not parse commit %s"),
5268
oid_to_hex(&commit->object.oid));
5269
goto unuse_commit_buffer;
5270
}
5271
find_commit_subject(p, &msg);
5272
if (write_message(msg, strlen(msg), path, 0)) {
5256
- res = error(_("could not write file: "
5273
+ ret = error(_("could not write file: "
5274
"'%s'"), path);
5275
goto unuse_commit_buffer;
5276
}
5277
+
5278
+ ret = 0;
5279
+
5280
unuse_commit_buffer:
5281
repo_unuse_commit_buffer(r, commit, p);
5262
- if (res)
5263
- return res;
5282
+ if (ret)
5283
+ goto out;
5284
}
5285
}
5286
5267
- strbuf_release(&rev);
5287
flags |= AMEND_MSG;
5288
}
5289
@@ -5272,18 +5291,29 @@ static int commit_staged_changes(struct repository *r,
5291
if (refs_ref_exists(get_main_ref_store(r),
5292
"CHERRY_PICK_HEAD") &&
5293
refs_delete_ref(get_main_ref_store(r), "",
5275
- "CHERRY_PICK_HEAD", NULL, REF_NO_DEREF))
5276
- return error(_("could not remove CHERRY_PICK_HEAD"));
5277
- if (unlink(git_path_merge_msg(r)) && errno != ENOENT)
5278
- return error_errno(_("could not remove '%s'"),
5279
- git_path_merge_msg(r));
5280
- if (!final_fixup)
5281
- return 0;
5294
+ "CHERRY_PICK_HEAD", NULL, REF_NO_DEREF)) {
5295
+ ret = error(_("could not remove CHERRY_PICK_HEAD"));
5296
+ goto out;
5297
+ }
5298
+
5299
+ if (unlink(git_path_merge_msg(r)) && errno != ENOENT) {
5300
+ ret = error_errno(_("could not remove '%s'"),
5301
+ git_path_merge_msg(r));
5302
+ goto out;
5303
+ }
5304
+
5305
+ if (!final_fixup) {
5306
+ ret = 0;
5307
+ goto out;
5308
+ }
5309
}
5310
5311
if (run_git_commit(final_fixup ? NULL : rebase_path_message(),
5285
- opts, flags))
5286
- return error(_("could not commit staged changes."));
5312
+ opts, flags)) {
5313
+ ret = error(_("could not commit staged changes."));
5314
+ goto out;
5315
+ }
5316
+
5317
unlink(rebase_path_amend());
5318
unlink(git_path_merge_head(r));
5319
refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
@@ -5301,7 +5331,12 @@ static int commit_staged_changes(struct repository *r,
5331
strbuf_reset(&ctx->current_fixups);
5332
ctx->current_fixup_count = 0;
5333
}
5304
- return 0;
5334
+
5335
+ ret = 0;
5336
+
5337
+out:
5338
+ strbuf_release(&rev);
5339
+ return ret;
5340
}
5341
5342
int sequencer_continue(struct repository *r, struct replay_opts *opts)