Hello John, see comments inline. >> @@ -22,6 +22,7 @@ SYNOPSIS >> [--color[=<when>] | --no-color] >> [--break] [--heading] [-p | --show-function] >> [-A <post-context>] [-B <pre-context>] [-C <context>] >> + [--threads <num>] > Is this the best place for this option? I know the current list isn't > sorted in any particular way, but here you're splitting up the set of > context options (`-A`, `-B`, `-C` and `-W`). Agree, I'll move the option both here and in documentation. >> -static int wait_all(void) >> +static int wait_all(struct grep_opt *opt) > I'm not sure passing a grep_opt in here is the cleanest way to do this. > Options are a UI concept and all we care about here is the number of > threads. > Since `threads` is a global, shouldn't the number of threads be a global > as well? Could we reuse `use_threads` here (possibly renaming it > `num_threads`)? This thought also crossed my mind, however we already pass grep_opt to start_threads() function, so I think passing it to wait_all() is not that ugly, and kind of symmetric. And I do not like the idea of duplicating same information in different places. What do you think? -- Best Regards, Victor -- 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