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

Bryan O'Sullivan bos at serpentine.com
Fri Jan 1 16:56:20 CST 2016


On Thu, Dec 31, 2015 at 11:05 AM, Augie Fackler <raf at 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 at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20160101/2ba193b2/attachment.html>


More information about the Mercurial-devel mailing list