Re: [PATCH 06/22] make sure partially read index is not changed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:

> A partially read index file currently cannot be written to disk.  Make
> sure that never happens, by re-reading the index file if the index file
> wasn't read completely before changing the in-memory index.

I am not quite sure what you are trying to do.  

In operations that modify the index (replace_index_entry(),
remove_index_entry_at(), etc.)  you lift the filter_ops and keep
partially_read flag still on.  In the write-out codepath, you have
an assert to make sure the caller has cleared the partially_read
flag.  A natural way to clear the flag is to re-read the index from
the file, but then you can easily lose the modifications.  Should
there be another safety that says "calling read_index() with the
partially_read flag on is a bug" or something?

Also shouldn't the flag be cleared upon discard_index()?  If it is
done there, you probably would not need to clear it in read_index().

>
> Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
> ---
>  builtin/update-index.c | 4 ++++
>  cache.h                | 4 +++-
>  read-cache-v2.c        | 3 +++
>  read-cache.c           | 8 ++++++++
>  4 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 5c7762e..03f6426 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -49,6 +49,8 @@ static int mark_ce_flags(const char *path, int flag, int mark)
>  	int namelen = strlen(path);
>  	int pos = cache_name_pos(path, namelen);
>  	if (0 <= pos) {
> +		if (active_cache_partially_read)
> +			cache_change_filter_opts(NULL);
>  		if (mark)
>  			active_cache[pos]->ce_flags |= flag;
>  		else
> @@ -253,6 +255,8 @@ static void chmod_path(int flip, const char *path)
>  	pos = cache_name_pos(path, strlen(path));
>  	if (pos < 0)
>  		goto fail;
> +	if (active_cache_partially_read)
> +		cache_change_filter_opts(NULL);
>  	ce = active_cache[pos];
>  	mode = ce->ce_mode;
>  	if (!S_ISREG(mode))
> diff --git a/cache.h b/cache.h
> index d38dfbd..f6c3407 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -293,7 +293,8 @@ struct index_state {
>  	struct cache_tree *cache_tree;
>  	struct cache_time timestamp;
>  	unsigned name_hash_initialized : 1,
> -		 initialized : 1;
> +		 initialized : 1,
> +		 partially_read : 1;
>  	struct hash_table name_hash;
>  	struct hash_table dir_hash;
>  	struct index_ops *ops;
> @@ -315,6 +316,7 @@ extern void free_name_hash(struct index_state *istate);
>  #define active_alloc (the_index.cache_alloc)
>  #define active_cache_changed (the_index.cache_changed)
>  #define active_cache_tree (the_index.cache_tree)
> +#define active_cache_partially_read (the_index.partially_read)
>  
>  #define read_cache() read_index(&the_index)
>  #define read_cache_from(path) read_index_from(&the_index, (path))
> diff --git a/read-cache-v2.c b/read-cache-v2.c
> index 1ed640d..2cc792d 100644
> --- a/read-cache-v2.c
> +++ b/read-cache-v2.c
> @@ -273,6 +273,7 @@ static int read_index_v2(struct index_state *istate, void *mmap,
>  		src_offset += 8;
>  		src_offset += extsize;
>  	}
> +	istate->partially_read = 0;
>  	return 0;
>  unmap:
>  	munmap(mmap, mmap_size);
> @@ -495,6 +496,8 @@ static int write_index_v2(struct index_state *istate, int newfd)
>  	struct stat st;
>  	struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
>  
> +	if (istate->partially_read)
> +		die("BUG: index: cannot write a partially read index");
>  	for (i = removed = extended = 0; i < entries; i++) {
>  		if (cache[i]->ce_flags & CE_REMOVE)
>  			removed++;
> diff --git a/read-cache.c b/read-cache.c
> index b30ee75..4529fab 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -30,6 +30,8 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
>  {
>  	struct cache_entry *old = istate->cache[nr];
>  
> +	if (istate->partially_read)
> +		index_change_filter_opts(istate, NULL);
>  	remove_name_hash(istate, old);
>  	set_index_entry(istate, nr, ce);
>  	istate->cache_changed = 1;
> @@ -467,6 +469,8 @@ int remove_index_entry_at(struct index_state *istate, int pos)
>  {
>  	struct cache_entry *ce = istate->cache[pos];
>  
> +	if (istate->partially_read)
> +		index_change_filter_opts(istate, NULL);
>  	record_resolve_undo(istate, ce);
>  	remove_name_hash(istate, ce);
>  	istate->cache_changed = 1;
> @@ -978,6 +982,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
>  {
>  	int pos;
>  
> +	if (istate->partially_read)
> +		index_change_filter_opts(istate, NULL);
>  	if (option & ADD_CACHE_JUST_APPEND)
>  		pos = istate->cache_nr;
>  	else {
> @@ -1184,6 +1190,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
>  				/* If we are doing --really-refresh that
>  				 * means the index is not valid anymore.
>  				 */
> +				if (istate->partially_read)
> +					index_change_filter_opts(istate, NULL);
>  				ce->ce_flags &= ~CE_VALID;
>  				istate->cache_changed = 1;
>  			}
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]