From: Elijah Newren <newren@gmail.com>
Date: Tue, 07 Sep 2010 03:06:13 GMT
Subject: Re: [RFC PATCH 07/15] cache_tree_update(): Capability to handle tree entries missing from index
Message-ID: <AANLkTinPyXCGWXH6VUtvTx6Cb4F5Bn238HQRE7tDZDtJ@mail.gmail.com>
In-Reply-To: <AANLkTinACDSruJXon0DkbU8V=ko55KzpVZmCRKA7i=5f@mail.gmail.com>

On Mon, Sep 6, 2010 at 7:28 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Sun, Sep 5, 2010 at 10:13 AM, Elijah Newren <newren@gmail.com> wrote:
>> +static void add_missing_paths_before(const char *path,
>> +                                    int pathlen,
>> +                                    int baselen,
>> +                                    struct tree_desc *desc,
>> +                                    struct strbuf *buffer)
>> +{
>> +       if (!git_sparse_pathspecs)
>> +               return; /* No paths are missing */
>> +
>> +       for (; desc->size; update_tree_entry(desc)) {
>> +               struct name_entry entry = desc->entry;
>> +               int entlen = strlen(entry.path);
>> +
>> +               /* We only want paths before path */
>> +               if (path)
>> +                       /*
>> +                        * FIXME: if entlen < pathlen, do I need to
>> +                        * use strncmp instead? Does entry.path being
>> +                        * NUL-terminated ensure memcmp exits
>> +                        * early?
>> +                        */
>> +                       if (memcmp(entry.path,
>> +                                  path+baselen,
>> +                                  pathlen-baselen) >= 0)
>> +                               break;
>
> If you do "break;" here, the current entry remains in "desc" and will
> be used to regenerate a duplicate tree in
> add_missing_paths_before(NULL, 0, 0, ..);. That entry is added the
> first time by update_one itself before the second add_missing() call.
> This fixes it for me
>
> diff --git a/cache-tree.c b/cache-tree.c
> index 79c28cc..1655738 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -278,17 +278,19 @@ static void add_missing_paths_before(const char *path,
>                int entlen = strlen(entry.path);
>
>                /* We only want paths before path */
> -               if (path)
> +               if (path) {
>                        /*
>                         * FIXME: if entlen < pathlen, do I need to
>                         * use strncmp instead? Does entry.path being
>                         * NUL-terminated ensure memcmp exits
>                         * early?
>                         */
> -                       if (memcmp(entry.path,
> -                                  path+baselen,
> -                                  pathlen-baselen) >= 0)
> +                       int result = memcmp(entry.path, path+baselen, pathlen-baselen);
> +                       if (result > 0)
>                                break;
> +                       if (!result)
> +                               continue;
> +               }
>
>                /* We only want paths "missing" from index due to sparsity */
>                if (path) {

Yes, good catch and thanks for the fix.