Re: [PATCH v2 7/8] python: Add failing tests for long disk path
by Nir Soffer
On Mon, Jun 17, 2019 at 5:30 PM Pavel Bar <pbar(a)redhat.com> wrote:
>
>
>
> On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof(a)gmail.com> wrote:
>
>> Sanlock allow up to 1023 characters in a disk path. We used to truncate
>> invalid long path silently instead of failing. Add failing tests for the
>> expected behaviour.
>>
>> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
>> ---
>> tests/constants.py | 4 +++
>> tests/python_test.py | 79 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 83 insertions(+)
>>
>> diff --git a/tests/constants.py b/tests/constants.py
>> index 4f45d13..2371fed 100644
>> --- a/tests/constants.py
>> +++ b/tests/constants.py
>> @@ -21,5 +21,9 @@ RINDEX_DISK_MAGIC = 0x01042018
>> # src/rindex_disk.h
>> # Copied from the docs module comment.
>>
>> RINDEX_ENTRY_SIZE = 64
>> RINDEX_ENTRIES_SECTORS = 2000
>> +
>> +# src/sanlock.h
>> +
>> +SANLK_PATH_LEN = 1024
>> diff --git a/tests/python_test.py b/tests/python_test.py
>> index cd8a82f..5624f60 100644
>> --- a/tests/python_test.py
>> +++ b/tests/python_test.py
>> @@ -550,10 +550,21 @@ def
>> test_write_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
>>
>> with raises_sanlock_errno():
>> sanlock.write_resource(b"ls_name", name, disks)
>>
>>
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_write_resource_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>>
>
> What about moving the "path" initialization line 2 lines up?
> Here and in many more places where "path" local variable is initialized
> and then passed to a method call.
> I know that from the *Java best practices* perspective it's better to
> wrap only code that can throw an exception with the exception handling
> block.
>
It is indeed better to move path out of the pytest.raises() block. However
it is
unlikely that we will fail with ValueError in that line.
>
>> + sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
>> +
>> +
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> @pytest.mark.parametrize("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)]
>> @@ -562,10 +573,21 @@ def
>> test_release_resource_parse_args(no_sanlock_daemon, name, filename, encoding
>>
>> with raises_sanlock_errno():
>> sanlock.release(b"ls_name", name, disks)
>>
>>
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_release_resource_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.release(b"ls_name", b"res_name", [(path, 0)])
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.release(b"ls_name", b"res_name", [(path, 0)])
>> +
>> +
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> @pytest.mark.parametrize("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)]
>> @@ -574,10 +596,21 @@ def
>> test_read_resource_owners_parse_args(no_sanlock_daemon, name, filename, enco
>>
>> with raises_sanlock_errno():
>> sanlock.read_resource_owners(b"ls_name", name, disks)
>>
>>
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_read_resource_owners_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.read_resource_owners(b"ls_name", b"res_name", [(path,
>> 0)])
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.read_resource_owners(b"ls_name", b"res_name", [(path,
>> 0)])
>> +
>> +
>> @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)
>>
>> @@ -624,10 +657,23 @@ 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)
>>
>> +
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_init_resource_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
>> +
>> + # init_resource access storage directly.
>> + with raises_sanlock_errno(errno.ENAMETOOLONG):
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
>> +
>> +
>> @pytest.mark.parametrize("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)
>> @@ -643,10 +689,21 @@ 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)
>>
>>
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_read_resource_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.read_resource(path)
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.read_resource(path)
>> +
>> +
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> @pytest.mark.parametrize("filename,encoding", FILE_NAMES)
>> def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
>> path = util.generate_path("/tmp/", filename, encoding)
>> disks = [(path, 0)]
>> @@ -656,10 +713,21 @@ def test_request_parse_args(no_sanlock_daemon,
>> name, filename, encoding):
>>
>> with raises_sanlock_errno():
>> sanlock.request(name, b"res_name", disks)
>>
>>
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_request_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.request(b"ls_name", b"res_name", [(path, 0)])
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.request(b"ls_name", b"res_name", [(path, 0)])
>> +
>> +
>> @pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
>> @pytest.mark.parametrize("filename,encoding", FILE_NAMES)
>> def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
>> path = util.generate_path("/tmp/", filename, encoding)
>> disks = [(path, 0)]
>> @@ -667,5 +735,16 @@ def test_acquire_parse_args(no_sanlock_daemon, name,
>> filename, encoding):
>> with raises_sanlock_errno():
>> sanlock.acquire(b"ls_name", name, disks, pid=os.getpid())
>>
>> with raises_sanlock_errno():
>> sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
>> +
>> +
>> +(a)pytest.mark.xfail(reason="path truncated silently")
>> +def test_acquire_path_length(no_sanlock_daemon):
>> + with pytest.raises(ValueError):
>> + path = "x" * constants.SANLK_PATH_LEN
>> + sanlock.acquire(b"ls_name", b"res_name", [(path, 0)],
>> pid=os.getpid())
>> +
>> + with raises_sanlock_errno():
>> + path = "x" * (constants.SANLK_PATH_LEN - 1)
>> + sanlock.acquire(b"ls_name", b"res_name", [(path, 0)],
>> pid=os.getpid())
>> --
>> 2.17.2
>>
>>
4 years, 5 months
[sanlock] 08/08: python: Fail with ValueError if disk path is too
long
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 53f7f0284c084ac2e4542fd1f71d0406075adb5d
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 22:36:39 2019 +0300
python: Fail with ValueError if disk path is too long
Previously if a path was too long, we truncated the path silently. This
probably fail when sanlock try to access non-existing path with "No such
path or directory" error. If you are unlucky, this could try to access
the wrong path, failing with bogus error, returning wrong data, or
worse, writing a resource into the wrong location, destroying existing
data.
Now we fail immediately with:
ValueError: Path is too long: '/too/long/path/...'
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 17 +++++++++++++++++
tests/python_test.py | 7 -------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index ca3713d..7c50aba 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -146,6 +146,17 @@ pystring_as_cstring(PyObject *obj)
}
static int
+validate_path(PyObject *path)
+{
+ if (PyBytes_Size(path) > SANLK_PATH_LEN - 1) {
+ set_error(PyExc_ValueError, "Path is too long: %s", path);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int
parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
{
int rv = 0;
@@ -163,6 +174,9 @@ parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
goto finally;
}
+ if (!validate_path(path))
+ goto finally;
+
strncpy(res_disk->path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
res_disk->offset = offset;
rv = 1;
@@ -656,6 +670,9 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
goto finally;
}
+ if (!validate_path(path))
+ goto finally;
+
/* prepare the resource disk path */
strncpy(res->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
diff --git a/tests/python_test.py b/tests/python_test.py
index 4ea1589..719bcb6 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -601,7 +601,6 @@ def test_write_resource_parse_args(
sanlock.write_resource(b"ls_name", name, disks)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_write_resource_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -625,7 +624,6 @@ def test_release_resource_parse_args(
sanlock.release(b"ls_name", name, disks)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_release_resource_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -649,7 +647,6 @@ def test_read_resource_owners_parse_args(
sanlock.read_resource_owners(b"ls_name", name, disks)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_read_resource_owners_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -712,7 +709,6 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.init_resource(name, b"res_name", disks)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_init_resource_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -745,7 +741,6 @@ def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
sanlock.read_resource(path)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_read_resource_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -769,7 +764,6 @@ def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.request(name, b"res_name", disks)
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_request_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
@@ -793,7 +787,6 @@ def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
-(a)pytest.mark.xfail(reason="path truncated silently")
def test_acquire_path_length(no_sanlock_daemon):
path = "x" * constants.SANLK_PATH_LEN
with pytest.raises(ValueError):
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 07/08: python: Add failing tests for long disk path
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit b0634b270bdeb88e2301859e46a80a9be95c89d3
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 22:33:16 2019 +0300
python: Add failing tests for long disk path
Sanlock allow up to 1023 characters in a disk path. We used to truncate
invalid long path silently instead of failing. Add failing tests for the
expected behaviour.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
tests/constants.py | 4 +++
tests/python_test.py | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+)
diff --git a/tests/constants.py b/tests/constants.py
index 3d9c332..4a98b3d 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -27,3 +27,7 @@ RINDEX_ENTRIES_SECTORS = 2000
# src/sanlock_rv.h
SANLK_LEADER_MAGIC = -223
+
+# src/sanlock.h
+
+SANLK_PATH_LEN = 1024
diff --git a/tests/python_test.py b/tests/python_test.py
index 86e60b1..4ea1589 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -601,6 +601,17 @@ def test_write_resource_parse_args(
sanlock.write_resource(b"ls_name", name, disks)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_write_resource_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.write_resource(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_release_resource_parse_args(
@@ -614,6 +625,17 @@ def test_release_resource_parse_args(
sanlock.release(b"ls_name", name, disks)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_release_resource_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.release(b"ls_name", b"res_name", [(path, 0)])
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.release(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_read_resource_owners_parse_args(
@@ -627,6 +649,17 @@ def test_read_resource_owners_parse_args(
sanlock.read_resource_owners(b"ls_name", name, disks)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_read_resource_owners_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.read_resource_owners(b"ls_name", b"res_name", [(path, 0)])
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.read_resource_owners(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
def test_get_hosts_parse_args(no_sanlock_daemon, name):
with raises_sanlock_errno():
@@ -679,6 +712,18 @@ def test_init_resource_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.init_resource(name, b"res_name", disks)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_init_resource_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
+
+ # init_resource access storage directly.
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno(errno.ENAMETOOLONG):
+ sanlock.init_resource(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_get_alignment_parse_args(no_sanlock_daemon, filename, encoding):
path = util.generate_path("/tmp/", filename, encoding)
@@ -700,6 +745,17 @@ def test_read_resource_parse_args(no_sanlock_daemon, filename, encoding):
sanlock.read_resource(path)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_read_resource_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.read_resource(path)
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.read_resource(path)
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
@@ -713,6 +769,17 @@ def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
sanlock.request(name, b"res_name", disks)
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_request_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.request(b"ls_name", b"res_name", [(path, 0)])
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.request(b"ls_name", b"res_name", [(path, 0)])
+
+
@pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
@@ -724,3 +791,14 @@ def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
with raises_sanlock_errno():
sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
+
+
+(a)pytest.mark.xfail(reason="path truncated silently")
+def test_acquire_path_length(no_sanlock_daemon):
+ path = "x" * constants.SANLK_PATH_LEN
+ with pytest.raises(ValueError):
+ sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
+
+ path = "x" * (constants.SANLK_PATH_LEN - 1)
+ with raises_sanlock_errno():
+ sanlock.acquire(b"ls_name", b"res_name", [(path, 0)], pid=os.getpid())
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 06/08: python: Add missing arg parsing tests
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 7874592fcd604e594d84e070496eedb0d04ed1de
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 22:30:56 2019 +0300
python: Add missing arg parsing tests
Add argument parsing tests for sanlock.acquire() and sanlock.request().
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
tests/python_test.py | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tests/python_test.py b/tests/python_test.py
index 0f70a6a..86e60b1 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -11,6 +11,7 @@ from __future__ import absolute_import
import errno
import io
+import os
import time
from contextlib import contextmanager
@@ -697,3 +698,29 @@ 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)
+
+
+(a)pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
+(a)pytest.mark.parametrize("filename,encoding", FILE_NAMES)
+def test_request_parse_args(no_sanlock_daemon, name, filename, encoding):
+ path = util.generate_path("/tmp/", filename, encoding)
+ disks = [(path, 0)]
+
+ with raises_sanlock_errno():
+ sanlock.request(b"ls_name", name, disks)
+
+ with raises_sanlock_errno():
+ sanlock.request(name, b"res_name", disks)
+
+
+(a)pytest.mark.parametrize("name", LOCKSPACE_OR_RESOURCE_NAMES)
+(a)pytest.mark.parametrize("filename,encoding", FILE_NAMES)
+def test_acquire_parse_args(no_sanlock_daemon, name, filename, encoding):
+ path = util.generate_path("/tmp/", filename, encoding)
+ disks = [(path, 0)]
+
+ with raises_sanlock_errno():
+ sanlock.acquire(b"ls_name", name, disks, pid=os.getpid())
+
+ with raises_sanlock_errno():
+ sanlock.acquire(name, b"res_name", disks, pid=os.getpid())
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 05/08: python: Replace memset() with initializer
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 44763e5669a1a758ec2f8e4cbc80969429d17a1c
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 21:16:13 2019 +0300
python: Replace memset() with initializer
C99 ensures that all fields of a struct are initialized to zero when
initializing to {0} and all characters are initialized to zero when
initializing to "";
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 36 ++++++++----------------------------
1 file changed, 8 insertions(+), 28 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 6824e01..ca3713d 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -343,14 +343,13 @@ py_get_alignment(PyObject *self __unused, PyObject *args)
{
int rv = -1;
PyObject *path = NULL;
- struct sanlk_disk disk;
+ struct sanlk_disk disk = {0};
/* parse python tuple */
if (!PyArg_ParseTuple(args, "O&", pypath_converter, &path)) {
goto finally;
}
- memset(&disk, 0, sizeof(struct sanlk_disk));
strncpy(disk.path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
/* get device alignment (gil disabled) */
@@ -420,14 +419,11 @@ 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;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
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&O&|kiii", kwlist,
convert_to_pybytes, &lockspace, pypath_converter, &path, &ls.host_id_disk.offset,
@@ -527,14 +523,11 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
uint32_t io_timeout = 0;
PyObject *lockspace = NULL;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
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&O&|kiIli", kwlist,
convert_to_pybytes, &lockspace, pypath_converter, &path, &ls.host_id_disk.offset,
@@ -585,14 +578,11 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
long align = ALIGNMENT_1M;
uint32_t io_timeout = 0;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
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, "O&|kli", kwlist,
pypath_converter, &path, &ls.host_id_disk.offset, &align, §or)) {
@@ -795,14 +785,11 @@ py_add_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
uint32_t iotimeout = 0;
PyObject *lockspace = NULL;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
static char *kwlist[] = {"lockspace", "host_id", "path", "offset",
"iotimeout", "wait", NULL};
- /* initialize lockspace structure */
- memset(&ls, 0, sizeof(struct sanlk_lockspace));
-
/* parse python tuple */
if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&kO&|kIi", kwlist,
convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
@@ -852,14 +839,11 @@ py_inq_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
int rv = BIND_ERROR, waitrs = 0, flags = 0;
PyObject *lockspace = NULL;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
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&kO&|ki", kwlist,
convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
@@ -916,14 +900,11 @@ py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds)
int wait = 1;
PyObject *lockspace = NULL;
PyObject *path = NULL;
- struct sanlk_lockspace ls;
+ struct sanlk_lockspace ls = {0};
static char *kwlist[] = {"lockspace", "host_id", "path", "offset",
"wait", "unused", NULL};
- /* initialize lockspace structure */
- memset(&ls, 0, sizeof(struct sanlk_lockspace));
-
/* parse python tuple */
if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&kO&|kii", kwlist,
convert_to_pybytes, &lockspace, &ls.host_id, pypath_converter, &path,
@@ -1407,7 +1388,7 @@ py_killpath(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
int rv = -1, num_args, sanlockfd = -1;
size_t kplen = 0;
- char kpargs[SANLK_HELPER_ARGS_LEN];
+ char kpargs[SANLK_HELPER_ARGS_LEN] = "";
PyObject *path = NULL;
PyObject *argslist;
@@ -1426,7 +1407,6 @@ py_killpath(PyObject *self __unused, PyObject *args, PyObject *keywds)
}
num_args = PyList_Size(argslist);
- memset(kpargs, 0, SANLK_HELPER_ARGS_LEN);
/* creating the arguments string from a python list */
for (int i = 0; i < num_args; i++) {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 04/08: python: Extract create_resource() helper
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit ca203015b1206616ba1a1734900346357acb75e2
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 21:01:56 2019 +0300
python: Extract create_resource() helper
Replace duplicate code for allocating and initializing sanlk_resource
struct with a helper.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 5c2fd1a..6824e01 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -172,25 +172,34 @@ finally:
return rv;
}
+static struct sanlk_resource *
+create_resource(int num_disks)
+{
+ size_t size = sizeof(struct sanlk_resource) +
+ sizeof(struct sanlk_disk) * num_disks;
+
+ struct sanlk_resource *res = calloc(1, size);
+ if (res == NULL) {
+ PyErr_NoMemory();
+ return NULL;
+ }
+
+ res->num_disks = num_disks;
+
+ return res;
+}
+
static int
parse_disks(PyObject *obj, struct sanlk_resource **res_ret)
{
- int num_disks, res_len;
+ int num_disks;
struct sanlk_resource *res;
num_disks = PyList_Size(obj);
- res_len = sizeof(struct sanlk_resource) +
- (sizeof(struct sanlk_disk) * num_disks);
- res = malloc(res_len);
-
- if (res == NULL) {
- PyErr_NoMemory();
+ res = create_resource(num_disks);
+ if (res == NULL)
return -1;
- }
-
- memset(res, 0, res_len);
- res->num_disks = num_disks;
for (int i = 0; i < num_disks; i++) {
PyObject *disk = PyList_GetItem(obj,i);
@@ -639,7 +648,7 @@ Sector can be one of (512, 4096).");
static PyObject *
py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv = -1, res_len, sector = SECTOR_SIZE_512;
+ int rv = -1, sector = SECTOR_SIZE_512;
long align = ALIGNMENT_1M;
PyObject *path = NULL;
struct sanlk_resource *res;
@@ -647,18 +656,9 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
static char *kwlist[] = {"path", "offset", "align", "sector", NULL};
- /* allocate the needed memory for the resource and one disk */
- res_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk);
- res = malloc(res_len);
-
- if (res == NULL) {
- PyErr_NoMemory();
+ res = create_resource(1 /* num_disks */);
+ if (res == NULL)
return NULL;
- }
-
- /* initialize resource and disk structures */
- memset(res, 0, res_len);
- res->num_disks = 1;
/* parse python tuple */
if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 03/08: python: Streamline initexception
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit f5c5b73895b68d8c6fd032a4fd5f98a9e5fb8a4a
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 20:15:36 2019 +0300
python: Streamline initexception
When we create the sanlock exception we create a function, create a
method from the function, create a dict from the method, and finally
create an exception from the dict. This simple flow does not require
goto for cleanup.
Creating a dict is simpler and less error prone using Py_BuildValue().
When we finish with unused pointers, it is safer to set them to NULL
using Py_CLEAR(), ensuring that we don't access freed memory.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 21a7a76..5c2fd1a 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1743,29 +1743,22 @@ sanlock_exception = {
static PyObject *
initexception(void)
{
- int rv;
- PyObject *dict, *func, *meth, *excp = NULL;
-
- if ((dict = PyDict_New()) == NULL)
- goto exit_fail;
-
- if ((func = PyCFunction_New(&sanlock_exception, NULL)) == NULL)
- goto exit_fail;
+ PyObject *func = PyCFunction_New(&sanlock_exception, NULL);
+ if (func == NULL)
+ return NULL;
- meth = PyObject_CallFunction((PyObject *) &PyProperty_Type, "O", func);
- Py_DECREF(func);
+ PyObject *meth = PyObject_CallFunction((PyObject *) &PyProperty_Type, "O", func);
+ Py_CLEAR(func);
if (meth == NULL)
- goto exit_fail;
-
- rv = PyDict_SetItemString(dict, sanlock_exception.ml_name, meth);
- Py_DECREF(meth);
- if (rv < 0)
- goto exit_fail;
+ return NULL;
- excp = PyErr_NewException("sanlock.SanlockException", NULL, dict);
+ PyObject *dict = Py_BuildValue("{s:O}", sanlock_exception.ml_name, meth);
+ Py_CLEAR(meth);
+ if (dict == NULL)
+ return NULL;
-exit_fail:
- Py_XDECREF(dict);
+ PyObject *excp = PyErr_NewException("sanlock.SanlockException", NULL, dict);
+ Py_CLEAR(dict);
return excp;
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 02/08: python: Use Py_CLEAR
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 304e928430ce3ce77985281cae4fb19905884552
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 19:50:44 2019 +0300
python: Use Py_CLEAR
Instead of:
Py_DECREF(obj);
obj = NULL;
We can use:
Py_CLEAR(obj);
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index e740f89..21a7a76 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1563,8 +1563,7 @@ py_get_event(PyObject *self __unused, PyObject *args)
if (PyList_Append(events, item) != 0)
goto exit_fail;
- Py_DECREF(item);
- item = NULL;
+ Py_CLEAR(item);
}
return events;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 01/08: python: Use Py_BuildValue to create event dicts
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit cacc09c02d2c0824ffbb0df9c48ecae703f8da60
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Sun Jun 16 19:44:43 2019 +0300
python: Use Py_BuildValue to create event dicts
Replace complex and error prone code to build event dict with
Py_BuildValue().
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 58 +++++++++-----------------------------------------------
1 file changed, 9 insertions(+), 49 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 9192395..e740f89 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -1527,7 +1527,6 @@ py_get_event(PyObject *self __unused, PyObject *args)
uint64_t from_generation;
PyObject *events = NULL;
PyObject *item = NULL;
- PyObject *value = NULL;
int rv;
if (!PyArg_ParseTuple(args, "i", &fd))
@@ -1549,55 +1548,16 @@ py_get_event(PyObject *self __unused, PyObject *args)
goto exit_fail;
}
- if ((item = PyDict_New()) == NULL)
- goto exit_fail;
+ item = Py_BuildValue(
+ "{s:K,s:K,s:K,s:K,s:K,s:K}",
+ "from_host_id", from_host_id,
+ "from_generation", from_generation,
+ "host_id", he.host_id,
+ "generation", he.generation,
+ "event", he.event,
+ "data", he.data);
- /* from_host_id */
- if ((value = PyLong_FromUnsignedLongLong(from_host_id)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "from_host_id", value);
- Py_DECREF(value);
- if (rv != 0)
- goto exit_fail;
-
- /* from_generation */
- if ((value = PyLong_FromUnsignedLongLong(from_generation)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "from_generation", value);
- Py_DECREF(value);
- if (rv != 0)
- goto exit_fail;
-
- /* host_id */
- if ((value = PyLong_FromUnsignedLongLong(he.host_id)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "host_id", value);
- Py_DECREF(value);
- if (rv != 0)
- goto exit_fail;
-
- /* generation */
- if ((value = PyLong_FromUnsignedLongLong(he.generation)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "generation", value);
- Py_DECREF(value);
- if (rv != 0)
- goto exit_fail;
-
- /* event */
- if ((value = PyLong_FromUnsignedLongLong(he.event)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "event", value);
- Py_DECREF(value);
- if (rv != 0)
- goto exit_fail;
-
- /* data */
- if ((value = PyLong_FromUnsignedLongLong(he.data)) == NULL)
- goto exit_fail;
- rv = PyDict_SetItemString(item, "data", value);
- Py_DECREF(value);
- if (rv != 0)
+ if (item == NULL)
goto exit_fail;
if (PyList_Append(events, item) != 0)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months
[sanlock] 14/14: python: Unify resource variables names
by pagure＠pagure.io
This is an automated email from the git hooks/post-receive script.
nsoffer pushed a commit to branch master
in repository sanlock.
commit 9ad356bbc2f411edd497a196206ae00efa2e2bd2
Author: Nir Soffer <nsoffer(a)redhat.com>
AuthorDate: Fri Jun 14 23:46:52 2019 +0300
python: Unify resource variables names
Some code was using "rs" and some "res". Unify all code to use "res".
This is also the term used in the tests.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
python/sanlock.c | 58 ++++++++++++++++++++++++++++----------------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c
index 12e2195..9192395 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -639,46 +639,46 @@ Sector can be one of (512, 4096).");
static PyObject *
py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
{
- int rv = -1, rs_len, sector = SECTOR_SIZE_512;
+ int rv = -1, res_len, sector = SECTOR_SIZE_512;
long align = ALIGNMENT_1M;
PyObject *path = NULL;
- struct sanlk_resource *rs;
- PyObject *rs_info = NULL;
+ struct sanlk_resource *res;
+ PyObject *res_info = NULL;
static char *kwlist[] = {"path", "offset", "align", "sector", NULL};
/* allocate the needed memory for the resource and one disk */
- rs_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk);
- rs = malloc(rs_len);
+ res_len = sizeof(struct sanlk_resource) + sizeof(struct sanlk_disk);
+ res = malloc(res_len);
- if (rs == NULL) {
+ if (res == NULL) {
PyErr_NoMemory();
return NULL;
}
/* initialize resource and disk structures */
- memset(rs, 0, rs_len);
- rs->num_disks = 1;
+ memset(res, 0, res_len);
+ res->num_disks = 1;
/* parse python tuple */
if (!PyArg_ParseTupleAndKeywords(args, keywds, "O&|kli", kwlist,
- pypath_converter, &path, &(rs->disks[0].offset), &align, §or)) {
+ pypath_converter, &path, &(res->disks[0].offset), &align, §or)) {
goto finally;
}
/* prepare the resource disk path */
- strncpy(rs->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
+ strncpy(res->disks[0].path, PyBytes_AsString(path), SANLK_PATH_LEN - 1);
/* set alignment/sector flags */
- if (add_align_flag(align, &rs->flags) == -1)
+ if (add_align_flag(align, &res->flags) == -1)
goto finally;
- if (add_sector_flag(sector, &rs->flags) == -1)
+ if (add_sector_flag(sector, &res->flags) == -1)
goto finally;
/* read sanlock resource (gil disabled) */
Py_BEGIN_ALLOW_THREADS
- rv = sanlock_read_resource(rs, 0);
+ rv = sanlock_read_resource(res, 0);
Py_END_ALLOW_THREADS
if (rv != 0) {
@@ -687,26 +687,26 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
}
/* prepare the dictionary holding the information */
- rs_info = Py_BuildValue(
+ res_info = Py_BuildValue(
#if PY_MAJOR_VERSION == 2
"{s:s,s:s,s:K}",
#else
"{s:y,s:y,s:K}",
#endif
- "lockspace", rs->lockspace_name,
- "resource", rs->name,
- "version", rs->lver);
- if (rs_info == NULL)
+ "lockspace", res->lockspace_name,
+ "resource", res->name,
+ "version", res->lver);
+ if (res_info == NULL)
goto finally;
finally:
- free(rs);
+ free(res);
Py_XDECREF(path);
if (rv != 0) {
- Py_XDECREF(rs_info);
+ Py_XDECREF(res_info);
return NULL;
}
- return rs_info;
+ return res_info;
}
/* write_resource */
@@ -726,7 +726,7 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
int rv = -1, max_hosts = 0, num_hosts = 0, clear = 0, sector = SECTOR_SIZE_512;
long align = ALIGNMENT_1M;
PyObject *lockspace = NULL, *resource = NULL;
- struct sanlk_resource *rs = NULL;
+ struct sanlk_resource *res = NULL;
PyObject *disks;
uint32_t flags = 0;
@@ -741,19 +741,19 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
}
/* parse and check sanlock resource */
- if (parse_disks(disks, &rs) < 0) {
+ if (parse_disks(disks, &res) < 0) {
goto finally;
}
/* prepare sanlock names */
- strncpy(rs->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
- strncpy(rs->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
+ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
+ strncpy(res->name, PyBytes_AsString(resource), SANLK_NAME_LEN);
/* set alignment/sector flags */
- if (add_align_flag(align, &rs->flags) == -1)
+ if (add_align_flag(align, &res->flags) == -1)
goto finally;
- if (add_sector_flag(sector, &rs->flags) == -1)
+ if (add_sector_flag(sector, &res->flags) == -1)
goto finally;
if (clear) {
@@ -762,7 +762,7 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
/* init sanlock resource (gil disabled) */
Py_BEGIN_ALLOW_THREADS
- rv = sanlock_write_resource(rs, max_hosts, num_hosts, flags);
+ rv = sanlock_write_resource(res, max_hosts, num_hosts, flags);
Py_END_ALLOW_THREADS
if (rv != 0) {
@@ -773,7 +773,7 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds)
finally:
Py_XDECREF(lockspace);
Py_XDECREF(resource);
- free(rs);
+ free(res);
if (rv != 0)
return NULL;
Py_RETURN_NONE;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
4 years, 5 months