On Mon, Sep 27, 2010 at 3:31 PM, Pat Thoyts <patthoyts@xxxxxxxxx> wrote: > On 27 September 2010 14:19, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote: >> On Fri, Sep 24, 2010 at 12:50 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>> Erik Faye-Lund <kusmabite@xxxxxxxxx> writes: >>> >>>> On Thu, Sep 23, 2010 at 10:35 AM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote: >>>>> Since open() already sets errno correctly for the NULL-case, let's just >>>>> avoid the problematic strcmp. >>>>> >>>>> Signed-off-by: Erik Faye-Lund <kusmabite@xxxxxxxxx> >>>> >>>> I guess I should add a comment as to why this patch is needed: >>>> >>>> This seems to be the culprit for issue 523 in the msysGit issue >>>> tracker: http://code.google.com/p/msysgit/issues/detail?id=523 >>>> >>>> fetch_and_setup_pack_index() apparently pass a NULL-pointer to >>>> parse_pack_index(), which in turn pass it to check_packed_git_idx(), >>>> which again pass it to open(). This all looks intentional to my >>>> (http.c-untrained) eye. >>> >>> Surely, open(NULL) should be rejected by a sane system, and your patch >>> looks sane to me. >>> >> >> Since this doesn't seem to be in git.git yet, perhaps you could squash >> this on top? I didn't notice it in time, but fopen lacked the same >> check (freopen already had the check). It's not as important, because >> it doesn't seem like we have any code reaching this path so far, but >> it would IMO be better to fix this now rather than having to chase >> down the issue again later... >> >> diff --git a/compat/mingw.c b/compat/mingw.c >> index 4595aaa..f069fea 100644 >> --- a/compat/mingw.c >> +++ b/compat/mingw.c >> @@ -160,7 +160,7 @@ ssize_t mingw_write(int fd, const void *buf, size_t count) >> #undef fopen >> FILE *mingw_fopen (const char *filename, const char *otype) >> { >> - if (!strcmp(filename, "/dev/null")) >> + if (filename && !strcmp(filename, "/dev/null")) >> filename = "nul"; >> return fopen(filename, otype); >> } >> > > I'll apply this to the devel branch and try to remember to squash it > on the next rebase-merge. > Cheers, > Pat > Wouldn't it be better to just get this squashed in git.git, and drop the patch in the next rebase-merge? Since this bug is in git.git as well, it makes sense to get the patch merged there, no? -- 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