Re: [PATCH] http: add_fill_function checks if function has been added

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Mar 8, 2009, 10:27 UTC
Message-ID: <be6fef0d0903080327u551c0b4mcb86f2ba76473efc@mail.gmail.com>
In-Reply-To: <7vd4ctm29m.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, Mar 8, 2009 at 5:49 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

> I didn't look at the callers of add_fill_function(), but "fill" takes a
> callback data and different invocation of add_fill_function() could be
> passing different callback data.  In such a case, doesn't it feel wrong to
> omit the "duplicated" calls to register the fill callback?  Your patch
> makes me suspect that it _might_ be better to fix the callers not to call
> the function repeatedly when they know they only want one-shot invocation.

Omitting duplicate calls in fill_active_slots does not mean that repeated calls of the fill functions won't take place.

In the current use instances in http-push and http-walker, run_active_slot and finish_all_active_slots (which calls run_active_slot) will always be called, because they are the functions that actually start the http requests[1], and that means the fill functions will be called repeated, because

run_active_slot, in its while loop, repeatedly calls
  step_active_slots calls
    fill_active_slots calls
      our fill functions.

fill_active_slots is the only direct caller of fill functions (duplicate ones included).

So we can be sure that the fill functions are called repeatedly (at least for the slot's active duration).

[1] Actually calling step_active_slots will start http requests, without you having to call run_active_slot.

-- 
Cheers,
Ray Chuan