Re: [msysGit] Re: [PATCH] mingw: do not crash on open(NULL, ...)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, 27 Sep 2010, Pat Thoyts 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.

Usually I mark commits like this with ("amend deadcafe") in the commit 
subject so I do not forget... ;-)

Ciao,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]