Re: [PATCH 1/2] Check order when reading index
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:20
Jaime Soriano Pastor [off-list ref] writes:
Subject: Re: [PATCH 1/2] Check order when reading index
Please be careful when crafting the commit title. This single line
will be the only one that readers will have to identify the change
among hundreds of entries in "git shortlog" output when trying to
see what kind of change went into the project during the given
period. Something like:
read_index_from(): catch out of order entries while reading an index file
perhaps?
quoted hunk
Signed-off-by: Jaime Soriano Pastor <redacted> --- read-cache.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)diff --git a/read-cache.c b/read-cache.c index 7f5645e..c1a9619 100644 --- a/read-cache.c +++ b/read-cache.c@@ -1438,6 +1438,21 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk, return ce; } +void check_ce_order(struct cache_entry *ce, struct cache_entry *next_ce)
Does this have to be global, i.e. not "static void ..."?
+{
+ int name_compare = strcmp(ce->name, next_ce->name);
+ if (0 < name_compare)
+ die("Unordered stage entries in index");
+ if (!name_compare) {
+ if (!ce_stage(ce))
+ die("Multiple stage entries for merged file '%s'",
+ ce->name);OK. If ce is at stage #0, no other entry can have the same name regardless of the stage, and next_ce having the same name violates that rule.
+ if (ce_stage(ce) >= ce_stage(next_ce))
+ die("Unordered stage entries for '%s'",
+ ce->name);Not quite. We do allow multiple higher stage entries; having two or more stage #1 entries is perfectly fine during a merge resolution, and both ce and next_ce may be pointing at the stage #1 entries of the same path. Replacing the comparison with ">" is sufficient, I think. Thanks.
quoted hunk
+ } +} + /* remember to discard_cache() before reading a different cache! */ int read_index_from(struct index_state *istate, const char *path) {@@ -1499,6 +1514,9 @@ int read_index_from(struct index_state *istate, const char *path) ce = create_from_disk(disk_ce, &consumed, previous_name); set_index_entry(istate, i, ce); + if (i > 0) + check_ce_order(istate->cache[i - 1], ce); + src_offset += consumed; } strbuf_release(&previous_name_buf);