Re: [PATCH v2 2/2] rebase: fix todo-list rereading

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/09/2021 17:13, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

.... Instead of relying on stat
data this patch simply reads the possibly edited todo list and
compares it to the original with memcmp(). This is much faster than
reparsing the todo list each time.

Nice.  Is that an egg of Columbus or what ;-)

+	if (strbuf_read_file_or_whine(&buf, get_todo_path(opts)) < 0)
+		return -1;
+	offset = get_item_line_offset(todo_list, todo_list->current + 1);
+	if (buf.len != todo_list->buf.len - offset ||
+	    memcmp(buf.buf, todo_list->buf.buf + offset, buf.len)) {
  		/* Reread the todo file if it has changed. */
  		todo_list_release(todo_list);
  		if (read_populate_todo(r, todo_list, opts))

As we already have the contents of hte file in the buffer, we could
further refactor the code around read_populate_todo() to tell it not
to reopen and reread the rebase-todo file (which risks toctou race),
but that is OK for now, I would think.

I did wonder about doing that but decided to punt it for now, I don't think the race is a concern - if another process is writing to the todo list while rebase is picking commits it is asking for trouble already. I suspect doing this will be easier once git-rebase--preserve-merges.sh is gone from master.

@@ -4271,6 +4267,7 @@ static int reread_todo_if_changed(struct repository *r,
  		/* `current` will be incremented on return */
  		todo_list->current = -1;
  	}
+	strbuf_release(&buf);
return 0;
  }
diff --git a/sequencer.h b/sequencer.h
index d57d8ea23d7..cdeb0c6be47 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -116,7 +116,6 @@ struct todo_list {
  	struct todo_item *items;
  	int nr, alloc, current;
  	int done_nr, total_nr;
-	struct stat_data stat;

Good riddance ;-)

Hear, hear!

Will queue.  Thanks.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux