From: Duy Nguyen Date: Sun, 14 Jul 2013 03:10:35 GMT Subject: Re: [PATCH v2 03/19] read-cache: move index v2 specific functions to their own file Message-ID: In-Reply-To: <1373650024-3001-4-git-send-email-t.gummerer@gmail.com> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer wrote: > @@ -489,8 +479,8 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig > #define CE_MATCH_RACY_IS_DIRTY 02 > /* do stat comparison even if CE_SKIP_WORKTREE is true */ > #define CE_MATCH_IGNORE_SKIP_WORKTREE 04 > -extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); > -extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); > +extern int ie_match_stat(struct index_state *, const struct cache_entry *, struct stat *, unsigned int); > +extern int ie_modified(struct index_state *, const struct cache_entry *, struct stat *, unsigned int); > I would rather we keep "const struct index_state*" if we could. I tried putting "const" back and found that ce_match_stat_basic() calls set_istate_ops(), which writes to "struct index_state". Putting set_istate_ops() in ce_match_stat_basic() may seem convenient, but does not make much sense (why would a match_stat function update index_ops?). I think you could move it out and - read_index calls set_istate_ops - (bonus) discard_index probably should reset "version" field to zero and clear internal_ops - the callers that use index without read_index must call initialize_index() or something, which in turn calls set_istate_ops. initialize_index() may take the preferred index version - do not let update-index modifies version field directly when --index-version is given. Wrap it with set_index_version() or something, so we can do internal conversion from one version to another if needed - remove set_istate_ops in write_index(), we may need internal_ops long before writing. When write_index is called, internal_ops should be already initialized -- Duy