Re: [PATCH 1/2] read_index_from(): catch out of order entries when reading an index file

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 29, 2014, 02:16 UTC
Message-ID: <CAPig+cS__Sw1gNj3Pz20OC51QBsuxBEXTzMStAerwfMuT2afQg@mail.gmail.com>
In-Reply-To: <1409168892-11345-1-git-send-email-jsorianopastor@gmail.com>

On Wed, Aug 27, 2014 at 3:48 PM, Jaime Soriano Pastor <jsorianopastor@gmail.com> wrote:

Show 25 quoted lines

> Signed-off-by: Jaime Soriano Pastor <jsorianopastor@gmail.com>
> ---
>  read-cache.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/read-cache.c b/read-cache.c
> index 7f5645e..1cdb762 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;
>  }
>
> +static void check_ce_order(struct cache_entry *ce, struct cache_entry *next_ce)
> +{
> +       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);
> +               if (ce_stage(ce) > ce_stage(next_ce))
> +                       die("Unordered stage entries for '%s'",
> +                               ce->name);

Perhaps consider dropping capitalization from error messages [1] (despite existing code in read-cache.c having a mix of the two styles). See "Error Messages" in Documentation/CodingGuidelines.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/251715/focus=253209

Show 18 quoted lines

> +       }
> +}
> +
>  /* 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);
> --
> 2.0.4.2.g7bc378e.dirty