rerere: check dirname format while iterating rr_cache directory
In rerere_gc(), we walk over the .git/rr_cache directory and create a
struct for each entry we find. We feed any name we get from readdir() to
find_rerere_dir(), which then calls get_sha1_hex() on it (since we use
the binary hash as a lookup key). If that fails (i.e., the directory
name is not what we expected), it returns NULL. But the comment in
find_rerere_dir() says "BUG".
It _would_ be a bug for the call from new_rerere_id_hex(), the only
other code path, to fail here; it's generating the hex internally. But
the call in rerere_gc() is using it say "is this a plausible directory
name".
Let's instead have rerere_gc() do its own "is this plausible" check.
That has two benefits:
  - we can now reliably BUG() inside find_rerere_dir(), which would
    catch bugs in the other code path (and we now will never return NULL
    from the function, which makes it easier to see that a rerere_id
    struct will always have a non-NULL "collection" field).
  - it makes the use of the binary hash an implementation detail of
    find_rerere_dir(), not known by callers. That will free us up to
    change it in a future patch.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
			
			
This commit is contained in:
		 Jeff King
					Jeff King
				
			
				
					committed by
					
						 Junio C Hamano
						Junio C Hamano
					
				
			
			
				
	
			
			
			 Junio C Hamano
						Junio C Hamano
					
				
			
						parent
						
							98c431b6f9
						
					
				
				
					commit
					2bc1a87e42
				
			
							
								
								
									
										14
									
								
								rerere.c
									
									
									
									
									
								
							
							
						
						
									
										14
									
								
								rerere.c
									
									
									
									
									
								
							| @ -146,7 +146,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex) | ||||
| 	int pos; | ||||
|  | ||||
| 	if (get_sha1_hex(hex, hash)) | ||||
| 		return NULL; /* BUG */ | ||||
| 		BUG("cannot parse rerere dir hex?"); | ||||
| 	pos = hash_pos(hash, rerere_dir, rerere_dir_nr, rerere_dir_hash); | ||||
| 	if (pos < 0) { | ||||
| 		rr_dir = xmalloc(sizeof(*rr_dir)); | ||||
| @ -1178,6 +1178,13 @@ static void prune_one(struct rerere_id *id, | ||||
| 		unlink_rr_item(id); | ||||
| } | ||||
|  | ||||
| /* Does the basename in "path" look plausibly like an rr-cache entry? */ | ||||
| static int is_rr_cache_dirname(const char *path) | ||||
| { | ||||
| 	unsigned char hash[GIT_MAX_RAWSZ]; | ||||
| 	return !get_sha1_hex(path, hash); | ||||
| } | ||||
|  | ||||
| void rerere_gc(struct repository *r, struct string_list *rr) | ||||
| { | ||||
| 	struct string_list to_remove = STRING_LIST_INIT_DUP; | ||||
| @ -1205,10 +1212,11 @@ void rerere_gc(struct repository *r, struct string_list *rr) | ||||
|  | ||||
| 		if (is_dot_or_dotdot(e->d_name)) | ||||
| 			continue; | ||||
| 		rr_dir = find_rerere_dir(e->d_name); | ||||
| 		if (!rr_dir) | ||||
| 		if (!is_rr_cache_dirname(e->d_name)) | ||||
| 			continue; /* or should we remove e->d_name? */ | ||||
|  | ||||
| 		rr_dir = find_rerere_dir(e->d_name); | ||||
|  | ||||
| 		now_empty = 1; | ||||
| 		for (id.variant = 0, id.collection = rr_dir; | ||||
| 		     id.variant < id.collection->status_nr; | ||||
|  | ||||
		Reference in New Issue
	
	Block a user