[PATCH 00/13] Fix path handling

[Nir Soffer]

Path arguments are handled now similar to the standard library,
supporting both bytes and Unicode paths. Unicode paths are encoded using
the file system encoding.

sanlock.killpath() argumnets are handled now in the same way as the path
argument, supporting non-ascii arguments.

For simplicity, python 2 version behave like python 3, supporting
Unicode paths with non-ascii content.

Amit Bawer (13):
  tests: Add path conversion tests to stub tests section
  python: Add pypath_converter
  python: Apply pypath_converter to parse_resource util function
  python: Apply pypath_converter to get_alignment API
  python: Add pypath_converter to init_lockspace API
  python: Apply pypath_converter to write_lockspace API
  python: Apply pypath_converter to read_lockspace API
  python: Apply pypath_converter to read_resource API
  python: Apply pypath_converter to add_lockspace
  python: Apply pypath_converter to inq_lockspace API
  python: Apply pypath_converter to rem_lockspace API
  python: Add pypath_converter to killpath API
  tests: Remove xfail marks from path file names

python/sanlock.c     | 335 ++++++++++++++++++++++++++++---------------
 tests/python_test.py |  94 ++++++++----
 2 files changed, 280 insertions(+), 149 deletions(-)

[PATCH 01/13] tests: Add path conversion tests to stub tests section

[Nir Soffer]

From: Amit Bawer

We add tests for path permutations involving xfail secnarios
to stub argument parsing tests. Also adding missing API tests.
Once we apply all patches involving the new path converter, we
expect all of them to pass with no fails in both python versions.
---
 tests/python_test.py | 82 ++++++++++++++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 21 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 926a0f0..5218435 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -517,64 +517,81 @@ def raises_sanlock_errno(expected_errno=errno.ECONNREFUSED):
         yield
     assert e.value.errno == expected_errno

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_rem_lockspace_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_rem_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
-        sanlock.rem_lockspace(name, 1, "ls_path", 0)
+        sanlock.rem_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_add_lockspace_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
-        sanlock.add_lockspace(name, 1, "ls_path", 0)
+        sanlock.add_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_write_lockspace_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
-        sanlock.write_lockspace(name, "ls_path")
+        sanlock.write_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_write_resource_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    disks = [(path, 0)]
     with raises_sanlock_errno():
-        sanlock.write_resource(name, b"res_name", [("disk_path",0)])
+        sanlock.write_resource(name, b"res_name", disks)

with raises_sanlock_errno():
-        sanlock.write_resource(b"ls_name", name, [("disk_path",0)])
+        sanlock.write_resource(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_release_resource_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    disks = [(path, 0)]
     with raises_sanlock_errno():
-        sanlock.release(name, b"res_name", [("disk_path",0)])
+        sanlock.release(name, b"res_name", disks)

with raises_sanlock_errno():
-        sanlock.release(b"ls_name", name, [("disk_path",0)])
+        sanlock.release(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_read_resource_owners_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    disks = [(path, 0)]
     with raises_sanlock_errno():
-        sanlock.read_resource_owners(name, b"res_name", [("disk_path",0)])
+        sanlock.read_resource_owners(name, b"res_name", disks)

with raises_sanlock_errno():
-        sanlock.read_resource_owners(b"ls_name", name, [("disk_path",0)])
+        sanlock.read_resource_owners(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
 def test_get_hosts_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.get_hosts(name, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_inq_lockspace_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_inq_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
-        sanlock.inq_lockspace(name, 1, "path", wait=False)
+        sanlock.inq_lockspace(name, 1, path, wait=False)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
 def test_reg_event_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
@@ -592,17 +609,40 @@ def test_set_event_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.set_event(name, 1, 1, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_init_lockspace_parse_args(no_sanlock_daemon, name):
[e-mail]("filename,encoding", FILE_NAMES)
+def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENODEV):
-        sanlock.init_lockspace(name, "path")
+        sanlock.init_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
-def test_init_resource_parse_args(no_sanlock_daemon, name):
-    disks = [("path", 0)]
[e-mail]("filename,encoding", FILE_NAMES)
+def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    disks = [(path, 0)]
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(b"ls_name", name, disks)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(name, b"res_name", disks)
+
[e-mail]("filename,encoding", FILE_NAMES)
+def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    with raises_sanlock_errno(errno.ENOENT):
+        sanlock.get_alignment(path)
+
[e-mail]("filename,encoding", FILE_NAMES)
+def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    with raises_sanlock_errno():
+        sanlock.read_lockspace(path)
+
[e-mail]("filename,encoding", FILE_NAMES)
+def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
+    path = util.generate_path("/tmp/", filename, encoding)
+    with raises_sanlock_errno():
+        sanlock.read_resource(path)
+

[PATCH 02/13] python: Add pypath_converter

[Nir Soffer]

From: Amit Bawer

In Python 3 we would like to handle paths as either unicode strings
or bytes. For this purpose we have a c-API called PyUnicode_FSConverter.
In this patch the missing API is implemented for Py2 based on Py3 [1]
and a unified no-cleanup behavior is enforced for Py3 path conversion as well.

[1] https://github.com/python/cpython/blob/master/Objects/unicodeobject.c#L3818
---
 python/sanlock.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/python/sanlock.c b/python/sanlock.c
index 39a9fb6..aaeaa86 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -71,10 +71,64 @@ __set_exception(int en, char *msg)
         PyErr_SetObject(py_exception, exc_tuple);
         Py_DECREF(exc_tuple);
     }
 }

+
+/*
+ * Converts a unicode path into PyBytes object.
+ * If conversion succeeds addr will hold a reference to a new
+ * PyBytes object containing bytes represenation of the system path
+ * given in arg object.
+ * Returns 1 on successful operation, 0 otherwise.
+ * Py2 implementation is based on Py3's PyUnicode_FSConverter[1].
+ * Py3 implementation wraps call PyUnicode_FSConverter and eliminates
+ * the cleanup support in order to make usage flow the same between
+ * versions.
+ * [1] https://github.com/python/cpython/blob/master/Objects/unicodeobject.c#L3818
+ */
+static int
+pypath_converter(PyObject* arg, void* addr)
+{
+    assert(arg && "path converter does not support cleanup (arg is NULL)");
+
+#if PY_MAJOR_VERSION == 2
+    /* python 2 implementation */
+    PyObject *output = NULL;
+    Py_ssize_t size;
+    const char *data;
+
+    if (PyBytes_Check(arg)) {
+        Py_INCREF(arg);
+        output = arg;
+    } else {
+        output = PyUnicode_AsEncodedString(arg, Py_FileSystemDefaultEncoding, NULL);
+        if (!output)
+            return 0;
+        assert(PyBytes_Check(output));
+    }
+
+    size = PyBytes_GET_SIZE(output);
+    data = PyBytes_AS_STRING(output);
+    if ((size_t)size != strlen(data)) {
+        PyErr_Format(PyExc_ValueError, "Embedded null byte");
+        Py_DECREF(output);
+        return 0;
+    }
+
+    *(PyObject**)addr = output;
+    return 1;
+#else
+    /* python 3 call wrapper */
+    int rv = PyUnicode_FSConverter(arg, addr);
+    /* python 2 does not suppot cleanups - same applies here */
+    if (rv == Py_CLEANUP_SUPPORTED)
+        rv = 1;
+    return rv;
+#endif
+}
+
 static uint64_t
 pyinteger_as_unsigned_long_long_mask(PyObject *obj)
 {
 #if PY_MAJOR_VERSION == 2
     return PyInt_AsUnsignedLongLongMask(obj);

[PATCH 03/13] python: Apply pypath_converter to parse_resource util function

[Nir Soffer]

From: Amit Bawer

We should be able to parse resource paths as either unicode or bytes.

Stub tests involving valid arguments parsing of resources paths
are set with additional permutations with no xfails expected.
---
 python/sanlock.c     | 46 ++++++++++++++++++++++++++++----------------
 tests/python_test.py | 16 +++++++++++----
 2 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index aaeaa86..1a983ee 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -157,10 +157,37 @@ pystring_as_cstring(PyObject *obj)
 #else
     return PyUnicode_AsUTF8(obj);
 #endif
 }

+static int
+parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
+{
+    int rv = 0;
+    PyObject *path = NULL;
+    uint64_t offset;
+
+    if (!PyTuple_Check(disk)) {
+         set_error(PyExc_ValueError, "Invalid disk %s", disk);
+         goto finally;
+    }
+
+    if (!PyArg_ParseTuple(disk, "O&K", pypath_converter, &path, &offset)) {
+        /* Override the error since it confusing in this context. */
+        set_error(PyExc_ValueError, "Invalid disk %s", disk);
+        goto finally;
+    }
+
+    strncpy(res_disk->path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
+    res_disk->offset = offset;
+    rv = 1;
+
+finally:
+    Py_XDECREF(path);
+    return rv;
+}
+
 static int
 __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
 {
     int i, num_disks, res_len;
     struct sanlk_resource *res;
@@ -178,30 +205,15 @@ __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++) {
-        PyObject *disk;
-        const char *path;
-        uint64_t offset;
+        PyObject *disk = PyList_GetItem(obj,i);

-        disk = PyList_GetItem(obj, i);
-
-        if (!PyTuple_Check(disk)) {
-            set_error(PyExc_ValueError, "Invalid disk %s", disk);
+        if (!parse_single_disk(disk, &(res->disks[i]))) {
             goto exit_fail;
-
         }
-
-        if (!PyArg_ParseTuple(disk, "sK", &path, &offset)) {
-            /* Override the error since it confusing in this context. */
-            set_error(PyExc_ValueError, "Invalid disk %s", disk);
-            goto exit_fail;
-        }
-
-        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 5218435..56d0c99 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -49,10 +49,18 @@ FILE_NAMES = [
         marks=pytest.mark.xfail(
             six.PY3,
             reason="currently not supporting bytes paths")),
 ]

+FILE_NAMES_NO_XFAILS = [
+    #name, encoding
+    ("ascii", None),
+    (u"ascii", None),
+    (u"\u05d0", None),
+    (u"\u05d0", "utf-8"),
+]
+
 LOCKSPACE_OR_RESOURCE_NAMES = [
     # Bytes are supported with python 2 and 3.
     pytest.param(b"\xd7\x90"),
     # Python 2 also supports str.
     pytest.param(
@@ -541,11 +549,11 @@ def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding)
     with raises_sanlock_errno():
         sanlock.write_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.write_resource(name, b"res_name", disks)
@@ -553,11 +561,11 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno():
         sanlock.write_resource(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.release(name, b"res_name", disks)
@@ -565,11 +573,11 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
     with raises_sanlock_errno():
         sanlock.release(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.read_resource_owners(name, b"res_name", disks)
@@ -617,11 +625,11 @@ def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno(errno.ENODEV):
         sanlock.init_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(b"ls_name", name, disks)

[PATCH 04/13] python: Apply pypath_converter to get_alignment API

[Nir Soffer]

From: Amit Bawer

We should be able to parse device path as either unicode or bytes.

New stub test was added for get_alignment API for path premutations
with no xfails expected.
---
 python/sanlock.c     | 16 ++++++++++------
 tests/python_test.py |  2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 1a983ee..334b237 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -350,32 +350,36 @@ get_alignment(path) -> int\n\
 Get device alignment.");

static PyObject *
 py_get_alignment(PyObject *self __unused, PyObject *args)
 {
-    int rv;
-    const char *path;
+    int rv = -1;
+    PyObject *path = NULL;
     struct sanlk_disk disk;

/* parse python tuple */
-    if (!PyArg_ParseTuple(args, "s", &path)) {
-        return NULL;
+    if (!PyArg_ParseTuple(args, "O&", pypath_converter, &path)) {
+        goto finally;
     }

memset(&disk, 0, sizeof(struct sanlk_disk));
-    strncpy(disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* get device alignment (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_direct_align(&disk);
     Py_END_ALLOW_THREADS

if (rv < 0) {
         __set_exception(rv, "Unable to get device alignment");
-        return NULL;
+        goto finally;
     }

+finally:
+    Py_XDECREF(path);
+    if (rv < 0)
+        return NULL;
     return Py_BuildValue("i", rv);
 }

/*
  * Convert parsed arg into PyBytes object.
diff --git a/tests/python_test.py b/tests/python_test.py
index 56d0c99..b251c07 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -634,11 +634,11 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(b"ls_name", name, disks)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(name, b"res_name", disks)

[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.get_alignment(path)

[PATCH 05/13] python: Add pypath_converter to init_lockspace API

[Nir Soffer]

From: Amit Bawer

We should be able to parse lockspace path as either unicode or bytes.

Stub test for init_lockspace was set with additional permutations
over lockspace path with no expected xfails.
---
 python/sanlock.c     | 11 ++++++-----
 tests/python_test.py |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 334b237..2b64b35 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -428,29 +428,29 @@ Initialize a device to be used as sanlock lockspace.");
 static PyObject *
 py_init_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv = -1, max_hosts = 0, num_hosts = 0, use_aio = 1;
     PyObject *lockspace = NULL;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;

static char *kwlist[] = {"lockspace", "path", "offset",
                                 "max_hosts", "num_hosts", "use_aio", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&s|kiii", kwlist,
-        convert_to_pybytes, &lockspace, &path, &ls.host_id_disk.offset, &max_hosts,
-        &num_hosts, &use_aio)) {
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&|kiii", kwlist,
+        convert_to_pybytes, &lockspace, pypath_converter, &path, &ls.host_id_disk.offset,
+        &max_hosts, &num_hosts, &use_aio)) {
         goto finally;
     }

/* prepare sanlock names */
     strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* init sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_direct_init(&ls, NULL, max_hosts, num_hosts, use_aio);
     Py_END_ALLOW_THREADS
@@ -460,10 +460,11 @@ py_init_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
         goto finally;
     }

finally:
     Py_XDECREF(lockspace);
+    Py_XDECREF(path);
     if (rv != 0)
         return NULL;
     Py_RETURN_NONE;
 }

diff --git a/tests/python_test.py b/tests/python_test.py
index b251c07..fb95977 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -617,11 +617,11 @@ def test_set_event_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.set_event(name, 1, 1, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENODEV):
         sanlock.init_lockspace(name, path)

[PATCH 06/13] python: Apply pypath_converter to write_lockspace API

[Nir Soffer]

From: Amit Bawer

We should be able to parse lockspace path as either unicode or bytes.

Stub for write_lockspace test was set with additional permutations
over paths with no expected xfails.
---
 python/sanlock.c     | 9 +++++----
 tests/python_test.py | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 2b64b35..6be0e64 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -535,29 +535,29 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv = -1, max_hosts = 0, sector = SECTOR_SIZE_512;
     long align = ALIGNMENT_1M;
     uint32_t io_timeout = 0;
     PyObject *lockspace = NULL;
-    const char  *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;

static char *kwlist[] = {"lockspace", "path", "offset", "max_hosts",
                                 "iotimeout", "align", "sector", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&s|kiIli", kwlist,
-        convert_to_pybytes, &lockspace, &path, &ls.host_id_disk.offset,
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O&|kiIli", kwlist,
+        convert_to_pybytes, &lockspace, pypath_converter, &path, &ls.host_id_disk.offset,
         &max_hosts, &io_timeout, &align, &sector)) {
         goto finally;
     }

/* prepare sanlock names */
     strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* set alignment/sector flags */
     if (add_align_flag(align, &ls.flags) == -1)
         goto finally;

@@ -574,10 +574,11 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
         goto finally;
     }

finally:
     Py_XDECREF(lockspace);
+    Py_XDECREF(path);
     if (rv != 0)
         return NULL;
     Py_RETURN_NONE;
 }

diff --git a/tests/python_test.py b/tests/python_test.py
index fb95977..23851df 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -541,11 +541,11 @@ def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno():
         sanlock.add_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.write_lockspace(name, path)

[PATCH 07/13] python: Apply pypath_converter to read_lockspace API

[Nir Soffer]

From: Amit Bawer

We should be able to parse lockspace path as either unicode or bytes.

New stub test for read_lockspace API was added with path permutations
with no xfails expected.
---
 python/sanlock.c     | 25 ++++++++++++++-----------
 tests/python_test.py |  2 +-
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 6be0e64..f5051b3 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -590,46 +590,46 @@ Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
 Sector can be one of (512, 4096).");

static PyObject *
 py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv, sector = SECTOR_SIZE_512;
+    int rv = -1, sector = SECTOR_SIZE_512;
     long align = ALIGNMENT_1M;
     uint32_t io_timeout = 0;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;
     PyObject *ls_info = NULL;

static char *kwlist[] = {"path", "offset", "align", "sector", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
-        &path, &ls.host_id_disk.offset, &align, &sector)) {
-        return NULL;
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
+        pypath_converter, &path, &ls.host_id_disk.offset, &align, &sector)) {
+        goto finally;
     }

/* prepare sanlock names */
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* set alignment/sector flags */
     if (add_align_flag(align, &ls.flags) == -1)
-        return NULL;
+        goto finally;

if (add_sector_flag(sector, &ls.flags) == -1)
-        return NULL;
+        goto finally;

/* read sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_read_lockspace(&ls, 0, &io_timeout);
     Py_END_ALLOW_THREADS

if (rv != 0) {
         __set_exception(rv, "Sanlock lockspace read failure");
-        return NULL;
+        goto finally;
     }

/* fill the information dictionary */
     ls_info = Py_BuildValue(
 #if PY_MAJOR_VERSION == 2
@@ -638,13 +638,16 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
             "{s:y,s:I}",
 #endif
             "lockspace", ls.name,
             "iotimeout", io_timeout);
     if (ls_info  == NULL)
-        return NULL;
+        goto finally;

-    /* success */
+finally:
+    Py_XDECREF(path);
+    if (rv != 0)
+        return NULL;
     return ls_info;
 }

/* read_resource */
 PyDoc_STRVAR(pydoc_read_resource, "\
diff --git a/tests/python_test.py b/tests/python_test.py
index 23851df..66e2b94 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -640,11 +640,11 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
 def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.get_alignment(path)

[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.read_lockspace(path)

[PATCH 08/13] python: Apply pypath_converter to read_resource API

[Nir Soffer]

From: Amit Bawer

We should be able to parse resource path as either unicode or bytes.

New stub test for read_resource API is added with path premutations
with no expected xfails.
---
 python/sanlock.c     | 33 ++++++++++++++++-----------------
 tests/python_test.py |  2 +-
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index f5051b3..a846570 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -657,13 +657,13 @@ Align can be one of (1048576, 2097152, 4194304, 8388608).\n\
 Sector can be one of (512, 4096).");

static PyObject *
 py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv, rs_len, sector = SECTOR_SIZE_512;
+    int rv = -1, rs_len, sector = SECTOR_SIZE_512;
     long align = ALIGNMENT_1M;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_resource *rs;
     PyObject *rs_info = NULL;

static char *kwlist[] = {"path", "offset", "align", "sector", NULL};

@@ -679,33 +679,33 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
     /* initialize resource and disk structures */
     memset(rs, 0, rs_len);
     rs->num_disks = 1;

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "s|kli", kwlist,
-        &path, &(rs->disks[0].offset), &align, &sector)) {
-        goto exit_fail;
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
+        pypath_converter, &path, &(rs->disks[0].offset), &align, &sector)) {
+        goto finally;
     }

/* prepare the resource disk path */
-    strncpy(rs->disks[0].path, path, SANLK_PATH_LEN - 1);
+    strncpy(rs->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* set alignment/sector flags */
     if (add_align_flag(align, &rs->flags) == -1)
-        goto exit_fail;
+        goto finally;

if (add_sector_flag(sector, &rs->flags) == -1)
-        goto exit_fail;
+        goto finally;

/* read sanlock resource (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_read_resource(rs, 0);
     Py_END_ALLOW_THREADS

if (rv != 0) {
         __set_exception(rv, "Sanlock resource read failure");
-        goto exit_fail;
+        goto finally;
     }

/* prepare the dictionary holding the information */
     rs_info = Py_BuildValue(
 #if PY_MAJOR_VERSION == 2
@@ -715,21 +715,20 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
 #endif
         "lockspace", rs->lockspace_name,
         "resource", rs->name,
         "version", rs->lver);
     if (rs_info  == NULL)
-        goto exit_fail;
+        goto finally;

-    /* success */
+finally:
     free(rs);
+    Py_XDECREF(path);
+    if (rv != 0) {
+        Py_XDECREF(rs_info);
+        return NULL;
+    }
     return rs_info;
-
-    /* failure */
-exit_fail:
-    free(rs);
-    Py_XDECREF(rs_info);
-    return NULL;
 }

/* write_resource */
 PyDoc_STRVAR(pydoc_write_resource, "\
 write_resource(lockspace, resource, disks, max_hosts=0, num_hosts=0, \
diff --git a/tests/python_test.py b/tests/python_test.py
index 66e2b94..3ce1301 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -646,11 +646,11 @@ def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
 def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.read_lockspace(path)

[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.read_resource(path)

[PATCH 09/13] python: Apply pypath_converter to add_lockspace

[Nir Soffer]

From: Amit Bawer

We should be able to parse lockspace path as either unicode or bytes.

New stub test is added for add_lockpace API with path permutation
with no xfails expected.
---
 python/sanlock.c     | 11 ++++++-----
 tests/python_test.py |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index a846570..4b7c810 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -811,34 +811,34 @@ static PyObject *
 py_add_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv = -1, async = 0, flags = 0;
     uint32_t iotimeout = 0;
     PyObject *lockspace = NULL;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;

static char *kwlist[] = {"lockspace", "host_id", "path", "offset",
                                 "iotimeout", "async", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&ks|kIi", kwlist,
-        convert_to_pybytes, &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset,
-        &iotimeout, &async)) {
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&kO&|kIi", kwlist,
+        convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
+        &ls.host_id_disk.offset, &iotimeout, &async)) {
         goto finally;
     }

/* prepare sanlock_add_lockspace flags */
     if (async) {
         flags |= SANLK_ADD_ASYNC;
     }

/* prepare sanlock names */
     strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* add sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_add_lockspace_timeout(&ls, flags, iotimeout);
     Py_END_ALLOW_THREADS
@@ -848,10 +848,11 @@ py_add_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
         goto finally;
     }

finally:
     Py_XDECREF(lockspace);
+    Py_XDECREF(path);
     if (rv != 0 )
         return NULL;
     Py_RETURN_NONE;
 }

diff --git a/tests/python_test.py b/tests/python_test.py
index 3ce1301..a9933f4 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -533,11 +533,11 @@ def test_rem_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno():
         sanlock.rem_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.add_lockspace(name, 1, path, 0)

[PATCH 10/13] python: Apply pypath_converter to inq_lockspace API

[Nir Soffer]

From: Amit Bawer

We would like to parse lockspace path as either unicode or bytes.

Stub test for inq_lockspace API is set with additional permutation
for paths with no xfails expected.
---
 python/sanlock.c     | 11 ++++++-----
 tests/python_test.py |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 4b7c810..e3b63a7 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -868,42 +868,43 @@ acquired or released.");
 static PyObject *
 py_inq_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv = BIND_ERROR, waitrs = 0, flags = 0;
     PyObject *lockspace = NULL;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;

static char *kwlist[] = {"lockspace", "host_id", "path", "offset",
                                 "wait", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&ks|ki", kwlist,
-        convert_to_pybytes, &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset,
-        &waitrs)) {
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&kO&|ki", kwlist,
+        convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
+        &ls.host_id_disk.offset, &waitrs)) {
         goto finally;
     }

/* prepare sanlock_inq_lockspace flags */
     if (waitrs) {
         flags |= SANLK_INQ_WAIT;
     }

/* prepare sanlock names */
     strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* add sanlock lockspace (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
     rv = sanlock_inq_lockspace(&ls, flags);
     Py_END_ALLOW_THREADS

finally:
     Py_XDECREF(lockspace);
+    Py_XDECREF(path);

if (rv == BIND_ERROR) {
         return NULL;
     } else if (rv == 0) {
         Py_RETURN_TRUE;
diff --git a/tests/python_test.py b/tests/python_test.py
index a9933f4..2be57e0 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -591,11 +591,11 @@ def test_get_hosts_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.get_hosts(name, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_inq_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.inq_lockspace(name, 1, path, wait=False)

[PATCH 11/13] python: Apply pypath_converter to rem_lockspace API

[Nir Soffer]

From: Amit Bawer

We would like to parse lockspace path as either unicode or bytes.

Stub test for rem_lockspace API is set with additional permutation
over path with no expected xfails.
---
 python/sanlock.c     | 10 ++++++----
 tests/python_test.py |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index e3b63a7..b7ffa28 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -931,29 +931,30 @@ successful termination these leases will be released.");
 static PyObject *
 py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
     int rv = -1, async = 0, unused = 0, flags = 0;
     PyObject *lockspace = NULL;
-    const char *path;
+    PyObject *path = NULL;
     struct sanlk_lockspace ls;

static char *kwlist[] = {"lockspace", "host_id", "path", "offset",
                                 "async", "unused", NULL};

/* initialize lockspace structure */
     memset(&ls, 0, sizeof(struct sanlk_lockspace));

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&ks|kii", kwlist,
-        convert_to_pybytes, &lockspace, &ls.host_id, &path, &ls.host_id_disk.offset,
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&kO&|kii", kwlist,
+        convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
+        &ls.host_id_disk.offset,
         &async, &unused)) {
         goto finally;
     }

/* prepare sanlock names */
     strncpy(ls.name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
-    strncpy(ls.host_id_disk.path, path, SANLK_PATH_LEN - 1);
+    strncpy(ls.host_id_disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);

/* prepare sanlock_rem_lockspace flags */
     if (async) {
         flags |= SANLK_REM_ASYNC;
     }
@@ -972,10 +973,11 @@ py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
         goto finally;
     }

finally:
     Py_XDECREF(lockspace);
+    Py_XDECREF(path);
     if (rv != 0)
         return NULL;
     Py_RETURN_NONE;
 }

diff --git a/tests/python_test.py b/tests/python_test.py
index 2be57e0..a23d2ec 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -525,11 +525,11 @@ def raises_sanlock_errno(expected_errno=errno.ECONNREFUSED):
         yield
     assert e.value.errno == expected_errno

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_rem_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.rem_lockspace(name, 1, path, 0)

[PATCH 12/13] python: Add pypath_converter to killpath API

[Nir Soffer]

From: Amit Bawer

We would like killpath to be able to handle paths as either unicode
and bytes, as well as the ones given in the command args.

Stub test for killpath API was modified to permutate over all paths
without expected xfails.
---
 python/sanlock.c     | 109 ++++++++++++++++++++++++++-----------------
 tests/python_test.py |   2 +-
 2 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index b7ffa28..d122c8e 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1361,10 +1361,55 @@ finally:
     if (rv != 0)
         return NULL;
     return ls_list;
 }

+static int
+parse_killpath_item(PyObject *item, char *kpargs, size_t *kplen)
+{
+    int rv = 0, i;
+    size_t arg_len;
+    PyObject *path = NULL;
+    const char *p = NULL;
+
+    if (!pypath_converter(item, &path)) {
+        goto finally;
+    }
+    p = PyBytes_AsString(path);
+    if (!p) {
+        goto finally;
+    }
+    /* computing the argument length considering the escape chars */
+    for (i = 0, arg_len = 0; p[i]; i++, arg_len++) {
+        if (p[i] == ' ' || p[i] == '\') arg_len++;
+    }
+
+    /* adding 2 for the space separator ' ' and the '\0' terminator */
+    if (*kplen + arg_len + 2 > SANLK_HELPER_ARGS_LEN) {
+        __set_exception(EINVAL, "Killpath arguments are too long");
+        goto finally;
+    }
+
+    /* adding the space separator between arguments */
+    if (*kplen > 0) {
+        kpargs[(*kplen)++] = ' ';
+    }
+
+    while (*p) {
+        if (*p == ' ' || *p == '\') {
+            kpargs[(*kplen)++] = '\';
+        }
+
+        kpargs[(*kplen)++] = *p++;
+    }
+    rv = 1;
+
+finally:
+    Py_XDECREF(path);
+    return rv;
+}
+
 /* killpath */
 PyDoc_STRVAR(pydoc_killpath, "\
 killpath(path, args [, slkfd=fd])\n\
 Configure the path and arguments of the executable used to fence a\n\
 process either by causing the pid to exit (kill) or putting it into\n\
@@ -1372,79 +1417,55 @@ a safe state (resources released).\n\
 The arguments must be in the format: ["arg1", "arg2", ...]");

static PyObject *
 py_killpath(PyObject *self __unused, PyObject *args, PyObject *keywds)
 {
-    int rv, i, j, n, num_args, sanlockfd = -1;
+    int rv = -1, i, num_args, sanlockfd = -1;
+    size_t kplen;
     char kpargs[SANLK_HELPER_ARGS_LEN];
-    const char *p, *path;
-    PyObject *argslist, *item;
+    PyObject *path = NULL;
+    PyObject *argslist;

static char *kwlist[] = {"path", "args", "slkfd", NULL};

/* parse python tuple */
-    if (!PyArg_ParseTupleAndKeywords(args, keywds, "sO!|i", kwlist,
-        &path, &PyList_Type, &argslist, &sanlockfd)) {
-        return NULL;
+    if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&O!|i", kwlist,
+        pypath_converter, &path, &PyList_Type, &argslist, &sanlockfd)) {
+        goto finally;
     }

/* checking the path length */
-    if (strlen(path) + 1 > SANLK_HELPER_PATH_LEN) {
+    if (PyBytes_Size(path) + 1 > SANLK_HELPER_PATH_LEN) {
         __set_exception(EINVAL, "Killpath path argument too long");
-        return NULL;
+        goto finally;
     }

num_args = PyList_Size(argslist);
     memset(kpargs, 0, SANLK_HELPER_ARGS_LEN);

/* creating the arguments string from a python list */
-    for (i = 0, n = 0; i < num_args; i++) {
-        size_t arg_len;
-
-        item = PyList_GetItem(argslist, i);
-        p = pystring_as_cstring(item);
-
-        if (p == NULL) {
-            __set_exception(EINVAL, "Killpath argument not a string");
-            return NULL;
-        }
-
-        /* computing the argument length considering the escape chars */
-        for (j = 0, arg_len = 0; p[j]; j++, arg_len++) {
-            if (p[j] == ' ' || p[j] == '\') arg_len++;
-        }
-
-        /* adding 2 for the space separator ' ' and the '\0' terminator */
-        if (n + arg_len + 2 > SANLK_HELPER_ARGS_LEN) {
-            __set_exception(EINVAL, "Killpath arguments are too long");
-            return NULL;
-        }
-
-        /* adding the space separator between arguments */
-        if (n > 0) {
-            kpargs[n++] = ' ';
-        }
-
-        while (*p) {
-            if (*p == ' ' || *p == '\') {
-                kpargs[n++] = '\';
-            }
-
-            kpargs[n++] = *p++;
+    for (i = 0, kplen = 0; i < num_args; i++) {
+        PyObject *item = PyList_GetItem(argslist, i);
+        if (!parse_killpath_item(item, kpargs, &kplen)) {
+            goto finally;
         }
     }

/* configure killpath (gil disabled) */
     Py_BEGIN_ALLOW_THREADS
-    rv = sanlock_killpath(sanlockfd, 0, path, kpargs);
+    rv = sanlock_killpath(sanlockfd, 0, PyBytes_AsString(path), kpargs);
     Py_END_ALLOW_THREADS

if (rv != 0) {
-        __set_exception(rv, "Killpath script not configured");
-        return NULL;
+       __set_exception(rv, "Killpath script not configured");
+       goto finally;
     }

+finally:
+    Py_XDECREF(path);
+    if (rv != 0)
+        return NULL;
     Py_RETURN_NONE;
 }

/* exception_errno */
 PyDoc_STRVAR(pydoc_errno, "exception errno");
diff --git a/tests/python_test.py b/tests/python_test.py
index a23d2ec..b072d6a 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -510,11 +510,11 @@ def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
     with pytest.raises(ValueError) as e:
         sanlock.write_resource(b"ls_name", b"res_name", disks)
     assert repr(disk) in str(e.value)

[e-mail]("filename,encoding", FILE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
 def test_killpath(tmpdir, sanlock_daemon, filename, encoding):
     cmd_path = util.generate_path(tmpdir, filename, encoding)
     fd = sanlock.register()
     sanlock.killpath(cmd_path, [cmd_path], fd)

[PATCH 13/13] tests: Remove xfail marks from path file names

[Nir Soffer]

From: Amit Bawer

At this point we expect all API calls to handle file paths
as either unicode strings or bytes. This applies to both python
versions.

XPASS/xfail expected for test_write_resource_4k_invalid_sector_size.
---
 tests/python_test.py | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index b072d6a..af75020 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -34,26 +34,10 @@ ALIGNMENT_2M = 2 * MiB
 SECTOR_SIZE_512 = 512
 SECTOR_SIZE_4K = 4 * KiB

FILE_NAMES = [
-    #name, encoding
-    ("ascii", None),
-    (u"ascii", None),
-    pytest.param(
-        u"\u05d0", None,
-        marks=pytest.mark.xfail(
-            six.PY2,
-            reason="currently not supporting non-ascii paths")),
-    pytest.param(
-        u"\u05d0", "utf-8",
-        marks=pytest.mark.xfail(
-            six.PY3,
-            reason="currently not supporting bytes paths")),
-]
-
-FILE_NAMES_NO_XFAILS = [
     #name, encoding
     ("ascii", None),
     (u"ascii", None),
     (u"\u05d0", None),
     (u"\u05d0", "utf-8"),
@@ -510,11 +494,11 @@ def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
     with pytest.raises(ValueError) as e:
         sanlock.write_resource(b"ls_name", b"res_name", disks)
     assert repr(disk) in str(e.value)

[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_killpath(tmpdir, sanlock_daemon, filename, encoding):
     cmd_path = util.generate_path(tmpdir, filename, encoding)
     fd = sanlock.register()
     sanlock.killpath(cmd_path, [cmd_path], fd)

@@ -525,35 +509,35 @@ def raises_sanlock_errno(expected_errno=errno.ECONNREFUSED):
         yield
     assert e.value.errno == expected_errno

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_rem_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.rem_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_add_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.add_lockspace(name, 1, path, 0)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_write_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.write_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.write_resource(name, b"res_name", disks)
@@ -561,11 +545,11 @@ def test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     with raises_sanlock_errno():
         sanlock.write_resource(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.release(name, b"res_name", disks)
@@ -573,11 +557,11 @@ def test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
     with raises_sanlock_errno():
         sanlock.release(b"ls_name", name, disks)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno():
         sanlock.read_resource_owners(name, b"res_name", disks)
@@ -591,11 +575,11 @@ def test_get_hosts_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.get_hosts(name, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_inq_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.inq_lockspace(name, 1, path, wait=False)

@@ -617,40 +601,40 @@ def test_set_event_parse_args(no_sanlock_daemon, name):
     with raises_sanlock_errno():
         sanlock.set_event(name, 1, 1, 1)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_init_lockspace_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENODEV):
         sanlock.init_lockspace(name, path)

@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     disks = [(path, 0)]
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(b"ls_name", name, disks)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.init_resource(name, b"res_name", disks)

[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno(errno.ENOENT):
         sanlock.get_alignment(path)

[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_read_lockspace_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.read_lockspace(path)

[e-mail]("filename,encoding", FILE_NAMES_NO_XFAILS)
[e-mail]("filename,encoding", FILE_NAMES)
 def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
     path = util.generate_path("/tmp/", filename, encoding)
     with raises_sanlock_errno():
         sanlock.read_resource(path)