Re: [Libguestfs] [PATCH v2] RHBZ#1406906: check return value of Python object functions

[Pino Toscano]

On Thursday, 11 May 2017 20:21:39 CEST Matteo Cafasso wrote:
> Verify the returned values of Python Object constructor functions
> are not NULL before adding them to a collection.
> 
> This is particularly relevant when constructing Unicode strings in
> Python 3 as they will return NULL if non UTF-8 characters are present.
> 
> Signed-off-by: Matteo Cafasso 
> ---

LGTM -- the only thing to change is the first line of the commit
message, which should be like "python: ... (RHBZ#...)".  Not going to
ask you to respin a v3 just because of that, though.

Amended, and pushed.  I will close the bug soon too.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2] RHBZ#1406906: check return value of Python object functions

[Matteo Cafasso]

Verify the returned values of Python Object constructor functions
are not NULL before adding them to a collection.

This is particularly relevant when constructing Unicode strings in
Python 3 as they will return NULL if non UTF-8 characters are present.

Signed-off-by: Matteo Cafasso 
---
 generator/python.ml| 102 ++---
 python/handle.c|  36 ---
 python/t/test830RHBZ1406906.py |  57 +++
 3 files changed, 152 insertions(+), 43 deletions(-)
 create mode 100644 python/t/test830RHBZ1406906.py

diff --git a/generator/python.ml b/generator/python.ml
index cf0829489..4cae24757 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -155,12 +155,20 @@ and generate_python_structs () =
 pr "PyObject *\n";
 pr "guestfs_int_py_put_%s_list (struct guestfs_%s_list *%ss)\n" typ typ 
typ;
 pr "{\n";
-pr "  PyObject *list;\n";
+pr "  PyObject *list, *element;\n";
 pr "  size_t i;\n";
 pr "\n";
 pr "  list = PyList_New (%ss->len);\n" typ;
-pr "  for (i = 0; i < %ss->len; ++i)\n" typ;
-pr "PyList_SetItem (list, i, guestfs_int_py_put_%s (&%ss->val[i]));\n" 
typ typ;
+pr "  if (list == NULL)\n";
+pr "return NULL;\n";
+pr "  for (i = 0; i < %ss->len; ++i) {\n" typ;
+pr "element = guestfs_int_py_put_%s (&%ss->val[i]);\n" typ typ;
+pr "if (element == NULL) {\n";
+pr "  Py_CLEAR (list);\n";
+pr "  return NULL;\n";
+pr "}\n";
+pr "PyList_SetItem (list, i, element);\n";
+pr "  }\n";
 pr "  return list;\n";
 pr "};\n";
 pr "#endif\n";
@@ -174,54 +182,72 @@ and generate_python_structs () =
   pr "PyObject *\n";
   pr "guestfs_int_py_put_%s (struct guestfs_%s *%s)\n" typ typ typ;
   pr "{\n";
-  pr "  PyObject *dict;\n";
+  pr "  PyObject *dict, *value;\n";
   pr "\n";
   pr "  dict = PyDict_New ();\n";
+  pr "  if (dict == NULL)\n";
+  pr "return NULL;\n";
   List.iter (
 function
 | name, FString ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "guestfs_int_py_fromstring (%s->%s));\n"
-  typ name
+pr "  value = guestfs_int_py_fromstring (%s->%s);" typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, FBuffer ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "guestfs_int_py_fromstringsize (%s->%s, 
%s->%s_len));\n"
-  typ name typ name
+pr "  value = guestfs_int_py_fromstringsize (%s->%s, 
%s->%s_len);\n"
+  typ name typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, FUUID ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "guestfs_int_py_fromstringsize (%s->%s, 
32));\n"
-  typ name
+pr "  value = guestfs_int_py_fromstringsize (%s->%s, 32);\n"
+  typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, (FBytes|FUInt64) ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "PyLong_FromUnsignedLongLong 
(%s->%s));\n"
-  typ name
+pr "  value = PyLong_FromUnsignedLongLong (%s->%s);\n" typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, FInt64 ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "PyLong_FromLongLong (%s->%s));\n"
-  typ name
+pr "  value = PyLong_FromLongLong (%s->%s);\n" typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, FUInt32 ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "PyLong_FromUnsignedLong (%s->%s));\n"
-  typ name
+pr "  value = PyLong_FromUnsignedLong (%s->%s);\n" typ name;
+pr "  if (value == NULL)\n";
+pr "goto err;\n";
+pr "  PyDict_SetItemString (dict, \"%s\", value);\n" name;
 | name, FInt32 ->
-pr "  PyDict_SetItemString (dict, \"%s\",\n" name;
-pr "PyLong_FromLong (%s->%s));\n"
-  typ name
+pr "  value = PyLong_FromLong (%s->%s);\n" typ name;
+pr "  if (value == NULL)\n";