Patchwork [v2] lazymanifest: check more return values in filtercopy

login
register
mail settings
Submitter Augie Fackler
Date Jan. 7, 2016, 12:13 a.m.
Message ID <d1f465b710bca4275136.1452125633@imladris.local>
Download mbox | patch
Permalink /patch/12583/
State Accepted
Headers show

Comments

Augie Fackler - Jan. 7, 2016, 12:13 a.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1451586702 18000
#      Thu Dec 31 13:31:42 2015 -0500
# Node ID d1f465b710bca42751366fcf975483aee3593b8d
# Parent  a23cb402be9b9e6812d084f0d30306c826b6d6a4
lazymanifest: check more return values in filtercopy

Spotted by Bryan O'Sullivan (and vexingly not the static analyzer I've
been using.)
Martin von Zweigbergk - Jan. 7, 2016, 12:33 a.m.
Pushed to the clowncopter, thanks!

On Wed, Jan 6, 2016 at 4:14 PM Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1451586702 18000
> #      Thu Dec 31 13:31:42 2015 -0500
> # Node ID d1f465b710bca42751366fcf975483aee3593b8d
> # Parent  a23cb402be9b9e6812d084f0d30306c826b6d6a4
> lazymanifest: check more return values in filtercopy
>
> Spotted by Bryan O'Sullivan (and vexingly not the static analyzer I've
> been using.)
>
> diff --git a/mercurial/manifest.c b/mercurial/manifest.c
> --- a/mercurial/manifest.c
> +++ b/mercurial/manifest.c
> @@ -707,11 +707,13 @@ static lazymanifest *lazymanifest_filter
>         copy->pydata = self->pydata;
>         Py_INCREF(self->pydata);
>         for (i = 0; i < self->numlines; i++) {
> -               PyObject *arg = PyString_FromString(self->lines[i].start);
> -               PyObject *arglist = PyTuple_Pack(1, arg);
> -               PyObject *result = PyObject_CallObject(matchfn, arglist);
> +               PyObject *arglist = NULL, *result = NULL;
> +               arglist = Py_BuildValue("(s)", self->lines[i].start);
> +               if (!arglist) {
> +                       return NULL;
> +               }
> +               result = PyObject_CallObject(matchfn, arglist);
>                 Py_DECREF(arglist);
> -               Py_DECREF(arg);
>                 /* if the callback raised an exception, just let it
>                  * through and give up */
>                 if (!result) {
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - Jan. 8, 2016, 1:27 p.m.
On 01/07/2016 12:13 AM, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <raf@durin42.com>
> # Date 1451586702 18000
> #      Thu Dec 31 13:31:42 2015 -0500
> # Node ID d1f465b710bca42751366fcf975483aee3593b8d
> # Parent  a23cb402be9b9e6812d084f0d30306c826b6d6a4
> lazymanifest: check more return values in filtercopy
>
> Spotted by Bryan O'Sullivan (and vexingly not the static analyzer I've
> been using.)

We should introduce the O'Sullivan test. You discuss about code 
correctness with something and if you confused your interlocutor with 
Bryan O'Sullivan while it was a static analyser, it passes the test.

Patch

diff --git a/mercurial/manifest.c b/mercurial/manifest.c
--- a/mercurial/manifest.c
+++ b/mercurial/manifest.c
@@ -707,11 +707,13 @@  static lazymanifest *lazymanifest_filter
 	copy->pydata = self->pydata;
 	Py_INCREF(self->pydata);
 	for (i = 0; i < self->numlines; i++) {
-		PyObject *arg = PyString_FromString(self->lines[i].start);
-		PyObject *arglist = PyTuple_Pack(1, arg);
-		PyObject *result = PyObject_CallObject(matchfn, arglist);
+		PyObject *arglist = NULL, *result = NULL;
+		arglist = Py_BuildValue("(s)", self->lines[i].start);
+		if (!arglist) {
+			return NULL;
+		}
+		result = PyObject_CallObject(matchfn, arglist);
 		Py_DECREF(arglist);
-		Py_DECREF(arg);
 		/* if the callback raised an exception, just let it
 		 * through and give up */
 		if (!result) {