Re: [PATCH v2 04/18] maintenance: initialize task array

From: Derrick Stolee <stolee@gmail.com>
Date: Jul 24, 2020, 12:51 UTC
Message-ID: <67e49b30-9a7f-8c18-3a06-96c516027c7a@gmail.com>
In-Reply-To: <1501583a-0fb0-ad88-bd2f-4f64c3c66980@gmail.com>

On 7/24/2020 8:23 AM, Derrick Stolee wrote:

Show 28 quoted lines

> On 7/23/2020 3:57 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> +static void initialize_tasks(void)
>>> +{
>>> +	int i;
>>> +	num_tasks = 0;
>>> +
>>> +	for (i = 0; i < MAX_NUM_TASKS; i++)
>>> +		tasks[i] = xcalloc(1, sizeof(struct maintenance_task));
>>> +
>>> +	tasks[num_tasks]->name = "gc";
>>> +	tasks[num_tasks]->fn = maintenance_task_gc;
>>> +	tasks[num_tasks]->enabled = 1;
>>> +	num_tasks++;
>>
>> Are we going to have 47 different tasks initialized by code like
>> this in the future?  I would have expected that you'd have a table
>> of tasks that serves as the blueprint copy and copy it to the table
>> to be used if there is some need to mutate the table-to-be-used.
> 
> Making it a table will likely make it easier to read. I hadn't
> thought of it.
> 
> At the start, I thought that the diff would look awful as we add
> members to the struct. However, the members that are not specified
> are set to zero, so I should be able to craft this into something
> not too terrible.

OK, my attempt has led to this final table:

	const struct maintenance_task default_tasks[] = {
		{
			"prefetch",
			maintenance_task_prefetch,
		},
		{
			"loose-objects",
			maintenance_task_loose_objects,
			loose_object_auto_condition,
		},
		{
			"incremental-repack",
			maintenance_task_incremental_repack,
			incremental_repack_auto_condition,
		},
		{
			"gc",
			maintenance_task_gc,
			need_to_gc,
			1,
		},
		{
			"commit-graph",
			maintenance_task_commit_graph,
			should_write_commit_graph,
		}
	};
	num_tasks = sizeof(default_tasks) / sizeof(struct maintenance_task);

This is followed by allocating and copying the data to the 'tasks' array, allowing it to be sorted and modified according to command-line arguments and config.

Is this what you intended?

Thanks, -Stolee