Hi Alban,
On Wed, 17 Mar 2021, Alban Gruin wrote:
> This moves the function add_cacheinfo() that already exists in
> update-index.c to update-index.c, renames it add_to_index_cacheinfo(),
> and adds an `istate' parameter. The new cache entry is returned through
> a pointer passed in the parameters. The return value is either 0
> (success), -1 (invalid path), or -2 (failed to add the file in the
> index).
This paragraph still talks about magic numbers, but the code has constants
for them. Maybe elevate the commit message to a more generic description
that does not spend time on specifying the exact values, but rather lists
the three outcomes in plain English?
Other than that, this looks fine to me! Thanks,
Dscho
>
> This will become useful in the next commit, when the three-way merge
> will need to call this function.
>
> Signed-off-by: Alban Gruin <alban.gruin@xxxxxxxxx>
> ---
> builtin/update-index.c | 25 +++++++------------------
> cache.h | 8 ++++++++
> read-cache.c | 35 +++++++++++++++++++++++++++++++++++
> 3 files changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 79087bccea..6b86e89840 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -404,27 +404,16 @@ static int process_path(const char *path, struct stat *st, int stat_errno)
> static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
> const char *path, int stage)
> {
> - int len, option;
> - struct cache_entry *ce;
> + int res;
>
> - if (!verify_path(path, mode))
> - return error("Invalid path '%s'", path);
> -
> - len = strlen(path);
> - ce = make_empty_cache_entry(&the_index, len);
> -
> - oidcpy(&ce->oid, oid);
> - memcpy(ce->name, path, len);
> - ce->ce_flags = create_ce_flags(stage);
> - ce->ce_namelen = len;
> - ce->ce_mode = create_ce_mode(mode);
> - if (assume_unchanged)
> - ce->ce_flags |= CE_VALID;
> - option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
> - option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
> - if (add_cache_entry(ce, option))
> + res = add_to_index_cacheinfo(&the_index, mode, oid, path, stage,
> + allow_add, allow_replace, NULL);
> + if (res == ADD_TO_INDEX_CACHEINFO_INVALID_PATH)
> + return error(_("Invalid path '%s'"), path);
> + if (res == ADD_TO_INDEX_CACHEINFO_UNABLE_TO_ADD)
> return error("%s: cannot add to the index - missing --add option?",
> path);
> +
> report("add '%s'", path);
> return 0;
> }
> diff --git a/cache.h b/cache.h
> index 6fda8091f1..41e30c0da2 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -832,6 +832,14 @@ int remove_file_from_index(struct index_state *, const char *path);
> int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
> int add_file_to_index(struct index_state *, const char *path, int flags);
>
> +#define ADD_TO_INDEX_CACHEINFO_INVALID_PATH (-1)
> +#define ADD_TO_INDEX_CACHEINFO_UNABLE_TO_ADD (-2)
> +
> +int add_to_index_cacheinfo(struct index_state *, unsigned int mode,
> + const struct object_id *oid, const char *path,
> + int stage, int allow_add, int allow_replace,
> + struct cache_entry **ce_ret);
> +
> int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
> int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
> void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
> diff --git a/read-cache.c b/read-cache.c
> index 1e9a50c6c7..b514523ca1 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1350,6 +1350,41 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
> return 0;
> }
>
> +int add_to_index_cacheinfo(struct index_state *istate, unsigned int mode,
> + const struct object_id *oid, const char *path,
> + int stage, int allow_add, int allow_replace,
> + struct cache_entry **ce_ret)
> +{
> + int len, option;
> + struct cache_entry *ce;
> +
> + if (!verify_path(path, mode))
> + return ADD_TO_INDEX_CACHEINFO_INVALID_PATH;
> +
> + len = strlen(path);
> + ce = make_empty_cache_entry(istate, len);
> +
> + oidcpy(&ce->oid, oid);
> + memcpy(ce->name, path, len);
> + ce->ce_flags = create_ce_flags(stage);
> + ce->ce_namelen = len;
> + ce->ce_mode = create_ce_mode(mode);
> + if (assume_unchanged)
> + ce->ce_flags |= CE_VALID;
> + option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
> + option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
> +
> + if (add_index_entry(istate, ce, option)) {
> + discard_cache_entry(ce);
> + return ADD_TO_INDEX_CACHEINFO_UNABLE_TO_ADD;
> + }
> +
> + if (ce_ret)
> + *ce_ret = ce;
> +
> + return 0;
> +}
> +
> /*
> * "refresh" does not calculate a new sha1 file or bring the
> * cache up-to-date for mode/content changes. But what it
> --
> 2.31.0
>
>
