Jonathan Nieder <jrnieder@xxxxxxxxx> writes: > Until we switched from using execvp to execve, the symptom was very > subtle: it only affected the error message when a program could not be > found, instead of affecting functionality more substantially. Hmph, what if you had bin/ssh/ directory and bin2/ssh executable and had bin:bin2 listed in this order in your $PATH? Without this change you'll get an error and that's the end of it. With this change, you'd be able to execute bin2/ssh executable, no? So I am not sure if I agree with the "this is just an error message subtlety". What does execvp() do when bin/ssh/ directory, bin2/ssh non-executable regular file, and bin3/ssh executable file exist and you have bin:bin2:bin3 on your $PATH? That is what locate_in_PATH() should emulate, I would think. >> + if (!stat(buf.buf, &st) && S_ISREG(st.st_mode)) >> return strbuf_detach(&buf, NULL); > > Should this share code with help.c's is_executable()? > > I suppose not, since that would have trouble finding scripts without > the executable bit set. > > I was momentarily nervous about what happens if this gets run on > Windows. This is just looking for a file's existence, not > executability, so it should be fine. When we are looking for "ssh" with locate_in_PATH(), shouldn't we look for "ssh.exe" on Windows, though?