On 11/15, Stefan Beller wrote: > > + /* > > + * Limit number of threads for child process to use. > > + * This is to prevent potential fork-bomb behavior of git-grep as each > > + * submodule process has its own thread pool. > > + */ > > + if (num_threads) > > + argv_array_pushf(&submodule_options, "--threads=%d", > > + (num_threads + 1) / 2); > > I think you would want to pass --threads=%d unconditionally, > as it also serves as a weak defusal for fork bombs. Is it possible to come here > with num_threads=0? (i.e. what happens if the user doesn't specify the number > of threads or such, do we fall back to some default or is it just 0?) > > I have seen some other places that check for num_threads unequal to 0, > as e.g. no mutex needs to be locked then (assuming we don't have any > thread but grep within the main process), but as you intend to use this also > as a helper to not blow up the number of threads recursively, we'd need to > pass at a number != 0 here? The option parsing logic in cmd_grep handles the cases where num_threads is some odd value (and fails if <0). In the case where it is 0, it will default to 8 under certain circumstances. I figured I would just let that logic handle the cases where num_theads ends up being 0 instead of explicitly passing threads=1. You can't pass threads=0 in some cases due to the default "oh look threads==0, looks like we should use 8!" case. > > > + > > + git grep -e "bar" --and -e "foo" --recurse-submodules > actual && > > nit here and in the tests below: > We prefer to have no white space between > and the file piped to. I'll fix that up everywhere. -- Brandon Williams