1:49 p.m.
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>
---
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
@@ -122,48 +122,30 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
memset(res, 0, res_len);
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;
return 0;
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
@@ -452,18 +452,26 @@ def test_write_resource_invalid_align_sector(
with pytest.raises(ValueError):
sanlock.write_resource(
"ls_name", "res_name", disks, align=align,
sector=sector)
-(a)pytest.mark.parametrize("resource", [
- "invalid resource tuple",
- b"invalid resource tuple",
+(a)pytest.mark.parametrize("disk", [
+ # 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)
--
2.17.2