[PATCH] Check order when reading index
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com
---
read-cache.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 7f5645e..e117d3a 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_next_ce(struct cache_entry *ce, struct cache_entry *next_ce) {
+ if (!ce || !next_ce)
+ return;
+ if (cache_name_compare(ce-name, ce_namelen(ce),
+ next_ce-name,
ce_namelen(next_ce)) 1)
+ die(Unordered stage entries in index);
+ if (ce_same_name(ce, next_ce)) {
+ if (!ce_stage(ce))
+ die(Multiple stage entries for merged file '%s',
+ ce-name);
+ if (ce_stage(ce) = ce_stage(next_ce))
+ die(Unordered stage entries for '%s', ce-name);
+ }
+}
+
/* 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_next_ce(istate-cache[i-1], ce);
+
src_offset += consumed;
}
strbuf_release(previous_name_buf);
--
2.0.4.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Check order when reading index
On Thu, Aug 21, 2014 at 3:43 PM, Jaime Soriano Pastor jsorianopas...@gmail.com wrote: + if (!ce_stage(ce)) + die(Multiple stage entries for merged file '%s', + ce-name); This case can be provoked by git update-index --index-info as shown in the patch with the added test, maybe it should be only a warning. And add too some variation of the patches in this thread to make the same command able to fix the situation. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Check order when reading index
On Thu, Aug 21, 2014 at 8:43 PM, Jaime Soriano Pastor jsorianopas...@gmail.com wrote: @@ -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_next_ce(istate-cache[i-1], ce); + src_offset += consumed; } strbuf_release(previous_name_buf); It may be nice to save the good index stamp as an index extension so we don't have to check this over and over. I'm thinking about big indexes where compare cost might matter (I'm not so sure yet, will do some testing when I have time). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Check order when reading index
Jaime Soriano Pastor jsorianopas...@gmail.com writes:
Signed-off-by: Jaime Soriano Pastor jsorianopas...@gmail.com
---
read-cache.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 7f5645e..e117d3a 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_next_ce(struct cache_entry *ce, struct cache_entry *next_ce) {
Have opening brace for the function on its own line, i.e.
void check_next_ce(struct cache_entry *ce, struct cache_entry *next_ce)
{
The function might be misnamed (see below), though.
+ if (!ce || !next_ce)
+ return;
Hmph, would it be either a programming error or a corrupt index
input to see a NULL in either of these variables?
+ if (cache_name_compare(ce-name, ce_namelen(ce),
+next_ce-name,
ce_namelen(next_ce)) 1)
An odd indentation that is overly deep to make it hard to read.
if (cache_name_compare(ce-name, ce_namelen(ce),
next_ce-name, ce_namelen(next_ce)) 1)
should be sufficient (replacing 7-SP before next_ce with a HT is OK
if the existing code nearby does so).
What is the significance of the return value from cache_name_compare()
that is strictly greater than 1 (as opposed to merely is it positive?)?
Perhaps you want something that is modeled after ce_same_name() that
ignores the stage, i.e.
int ce_name_compare(const struct cache_entry *a, const struct
cache_entry *b)
{
return strcmp(a-ce_name, b-ce_name);
}
without reimplementing the cache-name-compare() as
int bad_ce_same_name(const struct cache_entry *a, const struct
cache_entry *b)
{
return !ce_same_name(a, b);
}
to keep the two names with different length could never be the
same optimization.
- if (0 = ce_name_compare(ce, next)) then the names are not sorted
- if (!stage(ce) !name_compare(ce, next)) then the merged
entry 'ce' is not the only entry for the path
+ die(Unordered stage entries in index);
+ if (ce_same_name(ce, next_ce)) {
+ if (!ce_stage(ce))
+ die(Multiple stage entries for merged file '%s',
+ ce-name);
+ if (ce_stage(ce) = ce_stage(next_ce))
+ die(Unordered stage entries for '%s', ce-name);
+ }
+}
+
/* 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_next_ce(istate-cache[i-1], ce);
Have a SP each on both sides of binary operator -.
Judging from the way this helper function is used, it looks like
check_with_previous_ce() is a more appropriate name. After all, you
are not checking the next ce which you haven't even created yet ;-)
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
