Brad King <brad.king@xxxxxxxxxxx> writes:
> +extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh, int refresh_flags);
Why a new parameter? If refresh_flags can be ANY when refresh=NoThanks,
shouldn't they be a single variable that tells the callee how the entry
should be refreshed (e.g. "not at all", "normally", "missing is ok", etc.)?
> +static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really,
> + int flags)
> {
> - return refresh_cache_ent(&the_index, ce, really, NULL, NULL);
> + int not_new = (flags & REFRESH_IGNORE_MISSING) != 0;
> + int cache_errno = 0;
> + struct cache_entry *new;
> +
> + new = refresh_cache_ent(&the_index, ce, really, &cache_errno, NULL);
> +
> + if(!new && not_new && cache_errno == ENOENT)
> + return ce;
I think this is still one level too high in the abstraction chain.
"int really" might be of type signed int by historical accidents,
but it is "unsigned int options" for the underlying
refresh_cache_ent(). I'd suggest renaming this to "unsigned int
refresh_options" or something, and then define a new constatnt
similar to the existing CE_MATCH_IGNORE_*.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html