[sanlock] 18/23: python: Simplify parse_resource() with PyArg_ParseTuple

This is an automated email from the git hooks/post-receive script.

nsoffer pushed a commit to branch master
in repository sanlock.

commit 47eb3d9651b9c047d1ace2769a9f5daab3b4fb26
Author: Nir Soffer <nsoffer(a)redhat.com&gt;
AuthorDate: Fri May 17 17:46:08 2019 +0300

    python: Simplify parse_resource() with PyArg_ParseTuple
    
    Replace complex manual parsing code with se PyArg_ParseTuple, avoiding
    the backward compatibility issue, and greatly simplifying the code.
    
    The test for invalid resource was modifed to test more types and
    combinations, including incorrect number of items, and invalid types for
    the path and offset.
    
    Current status:
        - Py2: will build and pass tests.
        - Py3: will fail to build for missing respective API calls.
    
    Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
 python/sanlock.c     | 42 ++++++++++++------------------------------
 tests/python_test.py | 24 ++++++++++++++++--------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index b4533f2..5d45b6a 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -124,44 +124,26 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
     res->num_disks = num_disks;
 
     for (i = 0; i < num_disks; i++) {
-        const char *p = NULL;
-        PyObject *tuple, *path = NULL, *offset = NULL;
+        PyObject *disk;
+        const char *path;
+        uint64_t offset;
 
-        tuple = PyList_GetItem(obj, i);
+        disk = PyList_GetItem(obj, i);
 
-        if (PyTuple_Check(tuple)) {
-            if (PyTuple_Size(tuple) != 2) {
-                __set_exception(EINVAL, "Invalid resource tuple");
-                goto exit_fail;
-            }
-
-            path = PyTuple_GetItem(tuple, 0);
-            offset = PyTuple_GetItem(tuple, 1);
-
-            p = pystring_as_cstring(path);
-
-            if (!PyInt_Check(offset)) {
-                __set_exception(EINVAL, "Invalid resource offset");
-                goto exit_fail;
-            }
-        } else {
-            /* handle invalid non-tuple resource input */
-            set_value_error("invalid disk value: %s", tuple);
+        if (!PyTuple_Check(disk)) {
+            set_value_error("Invalid disk %s", disk);
             goto exit_fail;
+
         }
 
-        if (p == NULL) {
-            __set_exception(EINVAL, "Invalid resource path");
+        if (!PyArg_ParseTuple(disk, "sK", &path, &offset)) {
+            /* Override the error since it confusing in this context. */
+            set_value_error("Invalid disk %s", disk);
             goto exit_fail;
         }
 
-        strncpy(res->disks[i].path, p, SANLK_PATH_LEN - 1);
-
-        if (offset == NULL) {
-            res->disks[i].offset = 0;
-        } else {
-            res->disks[i].offset = PyInt_AsLong(offset);
-        }
+        strncpy(res->disks[i].path, path, SANLK_PATH_LEN - 1);
+        res->disks[i].offset = offset;
     }
 
     *res_ret = res;
diff --git a/tests/python_test.py b/tests/python_test.py
index a24990d..cfbdf9c 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -454,16 +454,24 @@ def test_write_resource_invalid_align_sector(
             "ls_name", "res_name", disks, align=align,
sector=sector)
 
 
-(a)pytest.mark.parametrize(&quot;resource&quot;, [
-    "invalid resource tuple",
-    b"invalid resource tuple",
+(a)pytest.mark.parametrize(&quot;disk&quot;, [
+    # Not a tuple - unicode and bytes:
+    "not a tuple",
+    b"not a tuple",
     u'\u05e9\u05dc\u05d5\u05dd',
-    b"\xd7\x90"
+    b"\xd7\x90",
+    # Tuple with incorrect length:
+    (),
+    ("path",),
+    ("path", 0, "extra"),
+    # Tuple with invalid content:
+    (0, "path"),
+    ("path", "not an offset"),
 ])
-def test_write_resource_invalid_path(tmpdir, sanlock_daemon, resource):
-    # Test parsing a resource which is not a list of tuples
-    disks = [resource]
+def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
+    # Test parsing disks list with invalid content.
+    disks = [disk]
     with pytest.raises(ValueError) as e:
         sanlock.write_resource("ls_name", "res_name", disks)
-    assert repr(resource) in str(e.value)
+    assert repr(disk) in str(e.value)
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.