Also, check if it fits in the temporary dir_buffer and can be chdir-ed into. Die for errors. Signed-off-by: Alex Riesen --- setup.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) On 8/3/07, Junio C Hamano wrote: > Alex Riesen writes: > > > Junio C Hamano, Thu, Aug 02, 2007 23:58:41 +0200: > >> "Alex Riesen" writes: > >> > + if (chdir(dir)) > >> > + rel = NULL; > > ... > >> > >> Shouldn't it die() instead, though? > > > > Dunno. Don't like dying. > > I do not understand your reasoning. Why is it better to use > mysteriously truncated path, which may result in doing something > the user did not ask you to, rather than saying "No, my > temporary buffer is not equipped to handle such an insanely long > pathname"? AFAIU, it is not only a truncated path which is a problem for chdir, but any failure to chdir, for any reason. And, if I understand set_work_tree returning NULL correctly (I assign rel NULL, which should be returned) - it is an error, and can be handled in the caller. But... Hmm... Looking at the code again, rel==NULL just means there is no prefix! You're right, better let it die. > >> Consolidating two of your patches, would this be Ok? > > > > Yes, but you may consider replacing strncpy with strlcpy: > > > >> + memcpy(dir_buffer, dir, len - suffix_len); > >> + dir_buffer[len - suffix_len] = '\0'; > > > > strlcpy(dir_buffer, dir, len - suffix_len + 1); > > Does that buy us that much? Before going to that codepath, we > have made sure the result fits, haven't we? No, we haven't. The code just checks if the given work tree path is longer than "/.git", to be able to cut off that safely.