Re: [PATCH 2/2] dir: test_one_path: fix inconsistent behavior due to missing '/'
Eric Sunshine wrote: Although undocumented, directory_exists_in_index_icase(dirname,len) unconditionally assumes the presence of a '/' at dirname[len] (despite being past the end-of-string). Callers are expected to respect [...] Fix this problem. So, does this fix the problem by changing directory_exists_in_index_icase() to be more liberal in what it accepts, or callers to be more conservative in what they pass in? Please forgive my laziness. I ask in order to save future readers the time of digging into the code. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] dir: test_one_path: fix inconsistent behavior due to missing '/'
On Sun, Aug 25, 2013 at 2:00 AM, Jonathan Nieder jrnie...@gmail.com wrote: Eric Sunshine wrote: Although undocumented, directory_exists_in_index_icase(dirname,len) unconditionally assumes the presence of a '/' at dirname[len] (despite being past the end-of-string). Callers are expected to respect [...] Fix this problem. So, does this fix the problem by changing directory_exists_in_index_icase() to be more liberal in what it accepts, or callers to be more conservative in what they pass in? It places the onus upon the caller. As mentioned in the cover letter [1], I was not happy with this solution. Junio felt likewise. A follow-up series [2] fixes both directory_exists_in_index() and directory_exists_in_index_icase() to be more liberal in what they accept, relieving the caller of the burden. By the time that series was posted, however, Junio and Peff had decided that a fix at a more fundamental level would be better (a conclusion with which I agree, but for which I do not yet have sufficient knowledge about git internals to implement). In the meantime, as an interim bug fix, Junio decided [3] to apply the patch to which you responded (but with an updated commit message). [1]: http://thread.gmane.org/gmane.comp.version-control.git/232796 [2]: http://thread.gmane.org/gmane.comp.version-control.git/232833 [3]: http://thread.gmane.org/gmane.comp.version-control.git/232833/focus=232837 Please forgive my laziness. I ask in order to save future readers the time of digging into the code. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] dir: test_one_path: fix inconsistent behavior due to missing '/'
Eric Sunshine sunsh...@sunshineco.com writes: ... A follow-up series [2] fixes both directory_exists_in_index() and directory_exists_in_index_icase() to be more liberal in what they accept, relieving the caller of the burden. By the time that series was posted, however, Junio and Peff had decided that a fix at a more fundamental level would be better (a conclusion with which I agree, Thanks for a summary; the last would be better is a qualified one, though. My understanding is that we agreed that it would be better *if* we can fix it at a more fundamental level. I looked at the codepaths involved just enough to make that off the cuff suggestion and no deeper than that, and I suspect neither did Peff. If any of us looked at the problem deep enough, we may realize that it would affect too many things, and $gmane/232833/focus=232835 (your second round) may turn out to be a better solution. I think none of us know yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
