Re: [PATCH 2/5] thread-utils: add a threaded task queue

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 28, 2015, 15:34 UTC
Message-ID: <xmqq1ten4eau.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <a35977776a7ea97b0ddd4d1921e3e95f@www.dscho.org>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

>> +void add_task(struct task_queue *tq,
>> +	      int (*fct)(struct task_queue *tq, void *task),
>
> Might make sense to typedef this... Maybe task_t?

Let's not introduce user defined type that ends with _t that is seen globally.

Show 11 quoted lines

>> +	      void *task)
>> +{
>> +	if (tq->early_return)
>> +		return;
>
> Ah, so "early_return" actually means "interrupted" or "canceled"?
>
> I guess I will have to set aside some time to wrap my head around the
> way tasks are handled here, in particular how the two `early_return`
> variables (`dispatcher()`'s local variable and the field in the
> task_queue`) interact.

We had a very similar conversation in $gmane/276324 as the early-return and get_task interaction was not quite intuitive.

I thought Stefan said something about this part of the logic being unreadable and needs rework. Perhaps that will come in the next reroll, or something?

I tend to agree with you that interrupted or cancelled would be a good name for this thing; at least it would help understanding what is going on than "early-return".

Thanks.