Patchwork [3,of,6,lazymanifest-errors] lazymanifest: check more error returns in setitem

login
register
mail settings
Submitter Augie Fackler
Date Dec. 31, 2015, 7:05 p.m.
Message ID <137312daf44138dfe058.1451588747@imladris.local>
Download mbox | patch
Permalink /patch/12451/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Augie Fackler - Dec. 31, 2015, 7:05 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1451587462 18000
#      Thu Dec 31 13:44:22 2015 -0500
# Node ID 137312daf44138dfe05826e7d928e345acf62a72
# Parent  2cde05d091f1e053f4cf57db579acdd98b8935cf
lazymanifest: check more error returns in setitem
Bryan O'Sullivan - Jan. 1, 2016, 10:56 p.m.
On Thu, Dec 31, 2015 at 11:05 AM, Augie Fackler <raf@durin42.com> wrote:

> diff --git a/mercurial/manifest.c b/mercurial/manifest.c
> --- a/mercurial/manifest.c
> +++ b/mercurial/manifest.c
> @@ -500,6 +500,9 @@ static int lazymanifest_setitem(
>         }
>
>         pyhash = PyTuple_GetItem(value, 0);
> +       if (!pyhash) {
> +               return -1;
> +       }
>

This is unnecessary. The only way it could fail is with a too-small tuple,
but you've already checked the size earlier.

Once again, this could be a PyTuple_GET_ITEM for speed.


>         if (!PyString_Check(pyhash)) {
>                 PyErr_Format(PyExc_TypeError,
>                              "node must be a 20-byte string");
> @@ -518,8 +521,14 @@ static int lazymanifest_setitem(
>                 return -1;
>         }
>         hash = PyString_AsString(pyhash);
> +       if (!hash) {
> +               return -1;
> +       }
>

Either the PyString_Check or the PyString_AsString is unnecessary here. See
previous comments.



>
>         pyflags = PyTuple_GetItem(value, 1);
> +       if (!pyflags) {
> +               return -1;
> +       }
>

Also unnecessary.


>         if (!PyString_Check(pyflags) || PyString_Size(pyflags) > 1) {
>

You could use PyString_GET_SIZE here.


>                 PyErr_Format(PyExc_TypeError,
>                              "flags must a 0 or 1 byte string");
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/manifest.c b/mercurial/manifest.c
--- a/mercurial/manifest.c
+++ b/mercurial/manifest.c
@@ -500,6 +500,9 @@  static int lazymanifest_setitem(
 	}
 
 	pyhash = PyTuple_GetItem(value, 0);
+	if (!pyhash) {
+		return -1;
+	}
 	if (!PyString_Check(pyhash)) {
 		PyErr_Format(PyExc_TypeError,
 			     "node must be a 20-byte string");
@@ -518,8 +521,14 @@  static int lazymanifest_setitem(
 		return -1;
 	}
 	hash = PyString_AsString(pyhash);
+	if (!hash) {
+		return -1;
+	}
 
 	pyflags = PyTuple_GetItem(value, 1);
+	if (!pyflags) {
+		return -1;
+	}
 	if (!PyString_Check(pyflags) || PyString_Size(pyflags) > 1) {
 		PyErr_Format(PyExc_TypeError,
 			     "flags must a 0 or 1 byte string");