git/sequencer.c, branch v2.50.1

Merge branch 'en/sequencer-comment-messages'

2025-05-27T20:59:11Z

Prefix '#' to the commit title in the "rebase -i" todo file, just like a merge commit being replayed. * en/sequencer-comment-messages: sequencer: make it clearer that commit descriptions are just comments

Merge branch 'js/misc-fixes'

2025-05-27T20:59:11Z

Assorted fixes for issues found with CodeQL. * js/misc-fixes: sequencer: stop pretending that an assignment is a condition bundle-uri: avoid using undefined output of `sscanf()` commit-graph: avoid using stale stack addresses trace2: avoid "futile conditional" Avoid redundant conditions fetch: avoid unnecessary work when there is no current branch has_dir_name(): make code more obvious upload-pack: rename `enum` to reflect the operation commit-graph: avoid malloc'ing a local variable fetch: carefully clear local variable's address after use commit: simplify code

Merge branch 'ly/sequencer-rearrange-leakfix'

2025-05-27T20:59:07Z

Leakfix. * ly/sequencer-rearrange-leakfix: sequencer: fix memory leak if `todo_list_rearrange_squash()` failed

Merge branch 'jk/oidmap-cleanup'

2025-05-19T23:02:47Z

Code cleanup. * jk/oidmap-cleanup: raw_object_store: drop extra pointer to replace_map oidmap: add size function oidmap: rename oidmap_free() to oidmap_clear()

Merge branch 'pw/sequencer-reflog-use-after-free'

2025-05-19T23:02:44Z

Use-after-free fix in the sequencer. * pw/sequencer-reflog-use-after-free: sequencer: rework reflog message handling sequencer: move reflog message functions

sequencer: make it clearer that commit descriptions are just comments

2025-05-16T19:28:27Z

Every once in a while, users report that editing the commit summaries in the todo list does not get reflected in the rebase operation, suggesting that users are (a) only using one-line commit messages, and (b) not understanding that the commit summaries are merely helpful comments to help them find the right hashes. It may be difficult to correct users' poor commit messages, but we can at least try to make it clearer that the commit summaries are not directives of some sort by inserting a comment character. Hopefully that leads to them looking a little further and noticing the hints at the bottom to use 'reword' or 'edit' directives. Yes, this change may look funny at first since it hardcodes '#' rather than using comment_line_str. However: * comment_line_str exists to allow disambiguation between lines in a commit message and lines that are instructions to users editing the commit message. No such disambiguation is needed for these comments that occur on the same line after existing directives * the exact "comment" character(s) on regular pick lines used aren't actually important; I could have used anything, including completely random variable length text for each line and it'd work because we ignore everything after 'pick' and the hash. * The whole point of this change is to signal to users that they should NOT be editing any part of the line after the hash (and if they do so, their edits will be ignored), while the whole point of comment_line_str is to allow highly flexible editing. So making it more general by using comment_line_str actually feels counterproductive. * The character for merge directives absolutely must be '#'; that has been deeply hardcoded for a long time (see below), and will break if some other comment character is used instead. In a desire to have pick and merge directives be similar, I use the same comment character for both. * Perhaps merge directives could be fixed to not be inflexible about the comment character used, if someone feels highly motivated, but I think that should be done in a separate follow-on patch. Here are (some of?) the locations where '#' has already been hardcoded for a long time for merges: 1) In check_label_or_ref_arg(): case TODO_LABEL: /* * '#' is not a valid label as the merge command uses it to * separate merge parents from the commit subject. */ 2) In do_merge(): /* * For octopus merges, the arg starts with the list of revisions to be * merged. The list is optionally followed by '#' and the oneline. */ merge_arg_len = oneline_offset = arg_len; for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) { if (!*p) break; if (*p == '#' && (!p[1] || isspace(p[1]))) { 3) In label_oid(): if ((buf->len == the_hash_algo->hexsz && !get_oid_hex(label, &dummy)) || (buf->len == 1 && *label == '#') || hashmap_get_from_hash(&state->labels, strihash(label), label)) { /* * If the label already exists, or if the label is a * valid full OID, or the label is a '#' (which we use * as a separator between merge heads and oneline), we * append a dash and a number to make it unique. */ Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

sequencer: fix memory leak if `todo_list_rearrange_squash()` failed

2025-05-15T20:53:33Z

In sequencer.c:todo_list_rearrange_squash, if it fails, memory allocated in `next`, `tail`, `subjects` and `subject2item` will leak. Jump to cleanup label before return could fix this leak problem. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano

sequencer: stop pretending that an assignment is a condition

2025-05-15T20:46:49Z

In 3e81bccdf3 (sequencer: factor out todo command name parsing, 2019-06-27), a `return` statement was introduced that basically was a long sequence of conditions, combined with `&&`, except for the last condition which is not really a condition but an assignment. The point of this construct was to return 1 (i.e. `true`) from the function if all of those conditions held true, and also assign the `bol` pointer to the end of the parsed command. Some static analyzers are really unhappy about such constructs. And human readers are at least puzzled, if not confused, by seeing a single `=` inside a chain of conditions where they would have expected to see `==` instead and, based on experience, immediately suspect a typo. Let's help all of this by turning this into the more verbose, more readable form of an `if` construct that both assigns the pointer as well as returns 1 if all of the conditions hold true. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano

oidmap: rename oidmap_free() to oidmap_clear()

2025-05-12T20:06:26Z

This function does not free the oidmap struct itself; it just drops all items from the map (using hashmap_clear_() internally). It should be called oidmap_clear(), per CodingGuidelines. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

sequencer: rework reflog message handling

2025-05-09T20:29:23Z

It has been reported that "git rebase --rebase-merges" can create corrupted reflog entries like e9c962f2ea0 HEAD@{8}: �: Merged in (pull request #4441) This is due to a use-after-free bug that happens because reflog_message() uses a static `struct strbuf` and is not called to update the current reflog message stored in `ctx->reflog_message` when creating the merge. This means `ctx->reflog_message` points to a stale reflog message that has been freed by subsequent call to reflog_message() by a command such as `reset` that used the return value directly rather than storing the result in `ctx->reflog_message`. Fix this by creating the reflog message nearer to where the commit is created and storing it in a local variable which is passed as an additional parameter to run_git_commit() rather than storing the message in `struct replay_ctx`. This makes it harder to forget to call `reflog_message()` before creating a commit and using a variable with a narrower scope means that a stale value cannot carried across a from one iteration of the loop to the next which should prevent any similar use-after-free bugs in the future. A existing test is modified to demonstrate that merges are now created with the correct reflog message. Reported-by: Kristoffer Haugsbakk Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano