On Thu, Oct 1, 2015 at 11:55 AM, Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> wrote: > Hi Stefan, > >> >> - if (!pp->get_next_task(pp->data, >> + if (!pp->get_next_task(&pp->children[i].data, >> &pp->children[i].process, >> - &pp->children[i].err)) >> + &pp->children[i].err, >> + pp->data)) >> return 1; > > ... the above hunk caught my eye. I don't know that it matters that > much, but since you have reordered parameters on some functions, should > pp->get_next_task() take the 'task_cb' as the last parameter, rather than > the first? > > I have not looked at the final result yet (just the interdiff), so please > just ignore the above if I've missed something obvious. :-D Well I reordered such that "passive" arguments come last, i.e. the cookies for consumption. In this specific case we ask get_next_task to fill in the cookie if desired. Unlike all the other cookie passing this is a double void pointer, so even syntactically we have a difference to other cookie passing around. If you look at the function definitions in the header, you find the arguments ordered as (Active/unique arguments for that function, child process, error buffer, cookies for consumption) That said, I find a few things I need to improve. pp->children[i].data, may want to be initialized to NULL before we ask get_next_task to fill in a cookie. If get_next_task decides not to, we have a clear default. The call to run_processes_parallel may be reordered to its original order again, as we pass in a cookie actively. (int n, int *cb, callbacks...) > > ATB, > Ramsay Jones > -- 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