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

Reply via email to