Tay Ray Chuan <rctay89@xxxxxxxxx> writes: > 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. That is not what I was worried about. There are two callsites to add_fill_function(). http-push.c:2439: add_fill_function(NULL, fill_active_slot); http-walker.c:936: add_fill_function(walker, (int (*)(void *)) fill_active_slot); The caller in http-push.c always passes NULL as the callback data for the fill_active_slot callback, which means that the callback function, as long as it does not depend on the state other than the callback function parameter (which is always NULL), will do the same thing whether you queue it only once or as many times as add_fill_function() was called, which the improvement your patch brings in. But it is not immediately obvious in http-walker.c callchain if multiple calls to add_fill_function(), if they were ever made, give the same "walker" as the callback data. Your patch only looks at the function but not the callback data for its filtering. Doesn't it mean that if a caller made two calls to add_fill_function() with walker#1 and then walker#2 as the callback data, you would discard the request for walker#2 and call the callback function fill_active_slot() with walker#1 inside run_active_slot repeatedly, without ever calling it for walker#2? I do not know if that actually happens in the current http-walker.c codepath, or the caller _happens to_ call the add_fill_function() only once and making my worry a non-issue, but as a patch submitter, you would know the answer to the question. If it is the former, then your change is introducing a bug. If it is the latter, your change won't break anything immediately, but still introduces a potential bug waiting to happen, as the API looks as if you can call add_fill_function() to ask for callback functions to be called with different callback data and the caller can rely on both of them to be called at appropriate times, but in reality that guarantee does not hold. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
