Merge branch 'kn/reflog-migration-fix-fix'
Fix bugs in an earlier attempt to fix "git refs migration". * kn/reflog-migration-fix-fix: refs/reftable: fix uninitialized memory access of `max_index` reftable: write correct max_update_index to headermaint
						commit
						1f124f3024
					
				
							
								
								
									
										7
									
								
								refs.c
								
								
								
								
							
							
						
						
									
										7
									
								
								refs.c
								
								
								
								
							|  | @ -1345,6 +1345,13 @@ int ref_transaction_update_reflog(struct ref_transaction *transaction, | ||||||
| 	update->flags &= ~REF_HAVE_OLD; | 	update->flags &= ~REF_HAVE_OLD; | ||||||
| 	update->index = index; | 	update->index = index; | ||||||
|  |  | ||||||
|  | 	/* | ||||||
|  | 	 * Reference backends may need to know the max index to optimize | ||||||
|  | 	 * their writes. So we store the max_index on the transaction level. | ||||||
|  | 	 */ | ||||||
|  | 	if (index > transaction->max_index) | ||||||
|  | 		transaction->max_index = index; | ||||||
|  |  | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
|  |  | ||||||
|  |  | ||||||
|  | @ -203,6 +203,7 @@ struct ref_transaction { | ||||||
| 	enum ref_transaction_state state; | 	enum ref_transaction_state state; | ||||||
| 	void *backend_data; | 	void *backend_data; | ||||||
| 	unsigned int flags; | 	unsigned int flags; | ||||||
|  | 	unsigned int max_index; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| /* | /* | ||||||
|  |  | ||||||
|  | @ -942,6 +942,7 @@ struct write_transaction_table_arg { | ||||||
| 	size_t updates_nr; | 	size_t updates_nr; | ||||||
| 	size_t updates_alloc; | 	size_t updates_alloc; | ||||||
| 	size_t updates_expected; | 	size_t updates_expected; | ||||||
|  | 	unsigned int max_index; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| struct reftable_transaction_data { | struct reftable_transaction_data { | ||||||
|  | @ -1019,6 +1020,7 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out, | ||||||
| 		arg->updates_nr = 0; | 		arg->updates_nr = 0; | ||||||
| 		arg->updates_alloc = 0; | 		arg->updates_alloc = 0; | ||||||
| 		arg->updates_expected = 0; | 		arg->updates_expected = 0; | ||||||
|  | 		arg->max_index = 0; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	arg->updates_expected++; | 	arg->updates_expected++; | ||||||
|  | @ -1428,7 +1430,6 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data | ||||||
| 	struct reftable_log_record *logs = NULL; | 	struct reftable_log_record *logs = NULL; | ||||||
| 	struct ident_split committer_ident = {0}; | 	struct ident_split committer_ident = {0}; | ||||||
| 	size_t logs_nr = 0, logs_alloc = 0, i; | 	size_t logs_nr = 0, logs_alloc = 0, i; | ||||||
| 	uint64_t max_update_index = ts; |  | ||||||
| 	const char *committer_info; | 	const char *committer_info; | ||||||
| 	int ret = 0; | 	int ret = 0; | ||||||
|  |  | ||||||
|  | @ -1438,7 +1439,12 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data | ||||||
|  |  | ||||||
| 	QSORT(arg->updates, arg->updates_nr, transaction_update_cmp); | 	QSORT(arg->updates, arg->updates_nr, transaction_update_cmp); | ||||||
|  |  | ||||||
| 	reftable_writer_set_limits(writer, ts, ts); | 	/* | ||||||
|  | 	 * During reflog migration, we add indexes for a single reflog with | ||||||
|  | 	 * multiple entries. Each entry will contain a different update_index, | ||||||
|  | 	 * so set the limits accordingly. | ||||||
|  | 	 */ | ||||||
|  | 	reftable_writer_set_limits(writer, ts, ts + arg->max_index); | ||||||
|  |  | ||||||
| 	for (i = 0; i < arg->updates_nr; i++) { | 	for (i = 0; i < arg->updates_nr; i++) { | ||||||
| 		struct reftable_transaction_update *tx_update = &arg->updates[i]; | 		struct reftable_transaction_update *tx_update = &arg->updates[i]; | ||||||
|  | @ -1540,12 +1546,6 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data | ||||||
| 				 */ | 				 */ | ||||||
| 				log->update_index = ts + u->index; | 				log->update_index = ts + u->index; | ||||||
|  |  | ||||||
| 				/* |  | ||||||
| 				 * Note the max update_index so the limit can be set later on. |  | ||||||
| 				 */ |  | ||||||
| 				if (log->update_index > max_update_index) |  | ||||||
| 					max_update_index = log->update_index; |  | ||||||
|  |  | ||||||
| 				log->refname = xstrdup(u->refname); | 				log->refname = xstrdup(u->refname); | ||||||
| 				memcpy(log->value.update.new_hash, | 				memcpy(log->value.update.new_hash, | ||||||
| 				       u->new_oid.hash, GIT_MAX_RAWSZ); | 				       u->new_oid.hash, GIT_MAX_RAWSZ); | ||||||
|  | @ -1609,8 +1609,6 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data | ||||||
| 	 * and log blocks. | 	 * and log blocks. | ||||||
| 	 */ | 	 */ | ||||||
| 	if (logs) { | 	if (logs) { | ||||||
| 		reftable_writer_set_limits(writer, ts, max_update_index); |  | ||||||
|  |  | ||||||
| 		ret = reftable_writer_add_logs(writer, logs, logs_nr); | 		ret = reftable_writer_add_logs(writer, logs, logs_nr); | ||||||
| 		if (ret < 0) | 		if (ret < 0) | ||||||
| 			goto done; | 			goto done; | ||||||
|  | @ -1632,6 +1630,8 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED, | ||||||
| 	int ret = 0; | 	int ret = 0; | ||||||
|  |  | ||||||
| 	for (size_t i = 0; i < tx_data->args_nr; i++) { | 	for (size_t i = 0; i < tx_data->args_nr; i++) { | ||||||
|  | 		tx_data->args[i].max_index = transaction->max_index; | ||||||
|  |  | ||||||
| 		ret = reftable_addition_add(tx_data->args[i].addition, | 		ret = reftable_addition_add(tx_data->args[i].addition, | ||||||
| 					    write_transaction_table, &tx_data->args[i]); | 					    write_transaction_table, &tx_data->args[i]); | ||||||
| 		if (ret < 0) | 		if (ret < 0) | ||||||
|  |  | ||||||
|  | @ -244,6 +244,18 @@ do | ||||||
| 	done | 	done | ||||||
| done | done | ||||||
|  |  | ||||||
|  | test_expect_success 'multiple reftable blocks with multiple entries' ' | ||||||
|  | 	test_when_finished "rm -rf repo" && | ||||||
|  | 	git init --ref-format=files repo && | ||||||
|  | 	test_commit -C repo first && | ||||||
|  | 	printf "create refs/heads/ref-%d HEAD\n" $(test_seq 5000) >stdin && | ||||||
|  | 	git -C repo update-ref --stdin <stdin && | ||||||
|  | 	test_commit -C repo second && | ||||||
|  | 	printf "update refs/heads/ref-%d HEAD\n" $(test_seq 3000) >stdin && | ||||||
|  | 	git -C repo update-ref --stdin <stdin && | ||||||
|  | 	test_migration repo reftable | ||||||
|  | ' | ||||||
|  |  | ||||||
| test_expect_success 'migrating from files format deletes backend files' ' | test_expect_success 'migrating from files format deletes backend files' ' | ||||||
| 	test_when_finished "rm -rf repo" && | 	test_when_finished "rm -rf repo" && | ||||||
| 	git init --ref-format=files repo && | 	git init --ref-format=files repo && | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Junio C Hamano
						Junio C Hamano