[PATCH 5 of 5 stable cpychecker] osutil: fix memory leak of PyBytes of path in statfiles

[Augie Fackler]

On Tue, Jan 27, 2015 at 05:20:51PM +0000, Martin von Zweigbergk wrote:
> This series looks good to me. I just have one question on the patch (see
> below).
>
> On Tue Jan 27 2015 at 7:30:58 AM Augie Fackler <raf at durin42.com> wrote:
>
> > # HG changeset patch
> > # User Augie Fackler <augie at google.com>
> > # Date 1422371836 18000
> > #      Tue Jan 27 10:17:16 2015 -0500
> > # Branch stable
> > # Node ID 88c751a71e730c029a12b3664befa94ed62a9b7c
> > # Parent  6c71c9d9336df3a27325b956aeb70fe6d2d953d1
> > osutil: fix memory leak of PyBytes of path in statfiles
> >
> > Spotted with cpychecker.
> >
> > diff --git a/mercurial/osutil.c b/mercurial/osutil.c
> > --- a/mercurial/osutil.c
> > +++ b/mercurial/osutil.c
> > @@ -410,17 +410,20 @@ static PyObject *statfiles(PyObject *sel
> >                 return NULL;
> >
> >         for (i = 0; i < count; i++) {
> > -               PyObject *stat;
> > +               PyObject *stat, *pypath;
> >                 struct stat st;
> >                 int ret, kind;
> >                 char *path;
> >
> > -               path = PyString_AsString(PySequence_GetItem(names, i));
> > +               pypath = PySequence_GetItem(names, i);
> > +               path = PyString_AsString(pypath);
> >                 if (path == NULL) {
> > +                       Py_DECREF(pypath);
> >
>
> What if pypath is NULL? Don't we need a separate check before creating
> 'path'?

Good catch. Mailed a v2.

>
> (Technically, it might be okay to attempt to create 'path' from NULL, but
> that might obscure the real error. We would also have to use XDECREF in
> that case.)
>
>
> >                         PyErr_SetString(PyExc_TypeError, "not a string");
> >                         goto bail;
> >                 }
> >                 ret = lstat(path, &st);
> > +               Py_DECREF(pypath);
> >                 kind = st.st_mode & S_IFMT;
> >                 if (ret != -1 && (kind == S_IFREG || kind == S_IFLNK)) {
> >                         stat = makestat(&st);
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
> >

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel