On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> @@ -238,7 +245,6 @@ static int read_index_v2(struct index_state *istate, void *mmap,
> disk_ce = (struct ondisk_cache_entry *)((char *)mmap + src_offset);
> ce = create_from_disk(disk_ce, &consumed, previous_name);
> set_index_entry(istate, i, ce);
> -
> src_offset += consumed;
> }
> strbuf_release(&previous_name_buf);
Nit pick. This is unnecessary.
> +int for_each_index_entry_v2(struct index_state *istate, each_cache_entry_fn fn, void *cb_data)
> +{
> + int i, ret = 0;
> + struct filter_opts *opts= istate->filter_opts;
Nitpick. space before "=".
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -996,6 +996,7 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
> memmove(istate->cache + pos + 1,
> istate->cache + pos,
> (istate->cache_nr - pos - 1) * sizeof(ce));
> +
> set_index_entry(istate, pos, ce);
> istate->cache_changed = 1;
> return 0;
NP: unnecessary change.
> +int set_internal_ops(struct index_state *istate)
> +{
> + if (!istate->internal_ops && istate->cache)
> + istate->internal_ops = &v2_internal_ops;
> + if (!istate->internal_ops)
> + return 0;
> + return 1;
> +}
> +
> +/*
> + * Execute fn for each index entry which is currently in istate. Data
> + * can be given to the function using the cb_data parameter.
> + */
> +int for_each_index_entry(struct index_state *istate, each_cache_entry_fn fn, void *cb_data)
> +{
> + if (!set_internal_ops(istate))
> + return 0;
> + return istate->internal_ops->for_each_index_entry(istate, fn, cb_data);
> }
set_internal_ops should have been called in read_index and initialize_index.
> @@ -1374,6 +1409,7 @@ int discard_index(struct index_state *istate)
> free(istate->cache);
> istate->cache = NULL;
> istate->cache_alloc = 0;
> + istate->filter_opts = NULL;
> return 0;
> }
Why is internal_ops not reset here?
> --- a/read-cache.h
> +++ b/read-cache.h
> @@ -24,11 +24,17 @@ struct cache_version_header {
> struct index_ops {
> int (*match_stat_basic)(const struct cache_entry *ce, struct stat *st, int changed);
> int (*verify_hdr)(void *mmap, unsigned long size);
> - int (*read_index)(struct index_state *istate, void *mmap, unsigned long mmap_size);
> + int (*read_index)(struct index_state *istate, void *mmap, unsigned long mmap_size,
> + struct filter_opts *opts);
> int (*write_index)(struct index_state *istate, int newfd);
> };
>
> +struct internal_ops {
> + int (*for_each_index_entry)(struct index_state *istate, each_cache_entry_fn fn, void *cb_data);
> +};
> +
I wonder if we do need separate internal_ops from index_ops. Why not
merge internal_ops in index_ops?
--
Duy
--
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