Lately I spent lot of time reviewing changes to the this messy code, and it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310 ++++++++++++++++++++----------------------- python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
We don't use use this tool, lets remove old craft.
If someone wants to add this back it must have proper documentation and integrated in the build system.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 7f0916a..7574141 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -14,31 +14,17 @@
#ifndef __unused #define __unused __attribute__ ((unused)) #endif
-#ifdef WITH_CPYCHECKER_SETS_EXCEPTION_ATTRIBUTE -#define __sets_exception \ - __attribute__((cpychecker_sets_exception)) -#else -#define __sets_exception -#endif - -#ifdef WITH_CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION_ATTRIBUTE -#define __neg_sets_exception \ - __attribute__((cpychecker_negative_result_sets_exception)) -#else -#define __neg_sets_exception -#endif - #define MODULE_NAME "sanlock"
#define BIND_ERROR -1000
/* Functions prototypes */ -static void __set_exception(int en, char *msg) __sets_exception; -static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret) __neg_sets_exception; +static void __set_exception(int en, char *msg); +static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret); static void set_error(PyObject *exception, const char* format, PyObject* obj);
/* Sanlock module */ PyDoc_STRVAR(pydoc_sanlock, "\ Copyright (C) 2010-2011 Red Hat, Inc. All rights reserved.\n\
Underscore prefix is reserved for the compiler and standard library so we should not use it for our helpers.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 58 ++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 7574141..42b46fb 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -19,11 +19,11 @@ #define MODULE_NAME "sanlock"
#define BIND_ERROR -1000
/* Functions prototypes */ -static void __set_exception(int en, char *msg); +static void set_sanlock_error(int en, char *msg); static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret); static void set_error(PyObject *exception, const char* format, PyObject* obj);
/* Sanlock module */ PyDoc_STRVAR(pydoc_sanlock, "\ @@ -34,11 +34,11 @@ of the GNU General Public License v2 or (at your option) any later version.");
/* Sanlock exception */ static PyObject *py_exception;
static void -__set_exception(int en, char *msg) +set_sanlock_error(int en, char *msg) { const char *err_name; PyObject *exc_tuple;
if (en < 0 && en > -200) { @@ -321,11 +321,11 @@ py_register(PyObject *self __unused, PyObject *args) Py_BEGIN_ALLOW_THREADS sanlockfd = sanlock_register(); Py_END_ALLOW_THREADS
if (sanlockfd < 0) { - __set_exception(sanlockfd, "Sanlock registration failed"); + set_sanlock_error(sanlockfd, "Sanlock registration failed"); return NULL; }
return Py_BuildValue("i", sanlockfd); } @@ -354,11 +354,11 @@ py_get_alignment(PyObject *self __unused, PyObject *args) Py_BEGIN_ALLOW_THREADS rv = sanlock_direct_align(&disk); Py_END_ALLOW_THREADS
if (rv < 0) { - __set_exception(rv, "Unable to get device alignment"); + set_sanlock_error(rv, "Unable to get device alignment"); goto finally; }
finally: Py_XDECREF(path); @@ -440,11 +440,11 @@ py_init_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_direct_init(&ls, NULL, max_hosts, num_hosts, use_aio); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock lockspace init failure"); + set_sanlock_error(rv, "Sanlock lockspace init failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -493,11 +493,11 @@ py_init_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_direct_init(NULL, res, max_hosts, num_hosts, use_aio); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock resource init failure"); + set_sanlock_error(rv, "Sanlock resource init failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -554,11 +554,11 @@ py_write_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_write_lockspace(&ls, max_hosts, 0, io_timeout); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock lockspace write failure"); + set_sanlock_error(rv, "Sanlock lockspace write failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -610,11 +610,11 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) 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"); + set_sanlock_error(rv, "Sanlock lockspace read failure"); goto finally; }
/* fill the information dictionary */ ls_info = Py_BuildValue( @@ -686,11 +686,11 @@ py_read_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_read_resource(rs, 0); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock resource read failure"); + set_sanlock_error(rv, "Sanlock resource read failure"); goto finally; }
/* prepare the dictionary holding the information */ rs_info = Py_BuildValue( @@ -770,11 +770,11 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_write_resource(rs, max_hosts, num_hosts, flags); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock resource write failure"); + set_sanlock_error(rv, "Sanlock resource write failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -829,11 +829,11 @@ py_add_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_add_lockspace_timeout(&ls, flags, iotimeout); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock lockspace add failure"); + set_sanlock_error(rv, "Sanlock lockspace add failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -899,11 +899,11 @@ finally: Py_RETURN_FALSE; } else if (rv == -EINPROGRESS) { Py_RETURN_NONE; }
- __set_exception(rv, "Sanlock lockspace inquire failure"); + set_sanlock_error(rv, "Sanlock lockspace inquire failure"); return NULL; }
/* rem_lockspace */ PyDoc_STRVAR(pydoc_rem_lockspace, "\ @@ -955,11 +955,11 @@ py_rem_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_rem_lockspace(&ls, flags); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock lockspace remove failure"); + set_sanlock_error(rv, "Sanlock lockspace remove failure"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -989,11 +989,11 @@ py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_get_lockspaces(&lss, &lss_count, 0); Py_END_ALLOW_THREADS
if (rv < 0) { - __set_exception(rv, "Sanlock get lockspaces failure"); + set_sanlock_error(rv, "Sanlock get lockspaces failure"); goto exit_fail; }
/* prepare the dictionary holding the information */ if ((ls_list = PyList_New(0)) == NULL) @@ -1067,11 +1067,11 @@ py_get_hosts(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_get_hosts(PyBytes_AsString(lockspace), host_id, &hss, &hss_count, 0); Py_END_ALLOW_THREADS
if (rv < 0) { - __set_exception(rv, "Sanlock get hosts failure"); + set_sanlock_error(rv, "Sanlock get hosts failure"); goto finally; }
ls_list = __hosts_to_list(hss, hss_count);
@@ -1111,11 +1111,11 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) goto finally; }
/* check if any of the slkfd or pid parameters was given */ if (sanlockfd == -1 && pid == -1) { - __set_exception(EINVAL, "Invalid slkfd and pid values"); + set_sanlock_error(EINVAL, "Invalid slkfd and pid values"); goto finally; }
/* parse and check sanlock resource */ if (__parse_resource(disks, &res) < 0) { @@ -1134,22 +1134,22 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) /* prepare the resource version */ if (version != Py_None) { res->flags |= SANLK_RES_LVER; res->lver = pyinteger_as_unsigned_long_long_mask(version); if (res->lver == (uint64_t)-1) { - __set_exception(EINVAL, "Unable to convert the version value"); + set_sanlock_error(EINVAL, "Unable to convert the version value"); goto finally; } }
/* acquire sanlock resource (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_acquire(sanlockfd, pid, 0, 1, &res, 0); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock resource not acquired"); + set_sanlock_error(rv, "Sanlock resource not acquired"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -1197,11 +1197,11 @@ py_release(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_release(sanlockfd, pid, 0, 1, &res); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock resource not released"); + set_sanlock_error(rv, "Sanlock resource not released"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -1254,22 +1254,22 @@ py_request(PyObject *self __unused, PyObject *args, PyObject *keywds) flags = SANLK_REQUEST_NEXT_LVER; } else { res->flags |= SANLK_RES_LVER; res->lver = pyinteger_as_unsigned_long_long_mask(version); if (res->lver == (uint64_t)-1) { - __set_exception(EINVAL, "Unable to convert the version value"); + set_sanlock_error(EINVAL, "Unable to convert the version value"); goto finally; } }
/* request sanlock resource (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_request(flags, action, res); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Sanlock request not submitted"); + set_sanlock_error(rv, "Sanlock request not submitted"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -1333,11 +1333,11 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd Py_BEGIN_ALLOW_THREADS rv = sanlock_read_resource_owners(res, 0, &hss, &hss_count); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Unable to read resource owners"); + set_sanlock_error(rv, "Unable to read resource owners"); goto finally; }
ls_list = __hosts_to_list(hss, hss_count);
@@ -1371,11 +1371,11 @@ parse_killpath_item(PyObject *item, char *kpargs, size_t *kplen) 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"); + set_sanlock_error(EINVAL, "Killpath arguments are too long"); goto finally; }
/* adding the space separator between arguments */ if (*kplen > 0) { @@ -1421,11 +1421,11 @@ py_killpath(PyObject *self __unused, PyObject *args, PyObject *keywds) goto finally; }
/* checking the path length */ if (PyBytes_Size(path) + 1 > SANLK_HELPER_PATH_LEN) { - __set_exception(EINVAL, "Killpath path argument too long"); + set_sanlock_error(EINVAL, "Killpath path argument too long"); goto finally; }
num_args = PyList_Size(argslist); memset(kpargs, 0, SANLK_HELPER_ARGS_LEN); @@ -1442,11 +1442,11 @@ py_killpath(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_killpath(sanlockfd, 0, PyBytes_AsString(path), kpargs); Py_END_ALLOW_THREADS
if (rv != 0) { - __set_exception(rv, "Killpath script not configured"); + set_sanlock_error(rv, "Killpath script not configured"); goto finally; }
finally: Py_XDECREF(path); @@ -1493,11 +1493,11 @@ py_reg_event(PyObject *self __unused, PyObject *args) Py_BEGIN_ALLOW_THREADS fd = sanlock_reg_event(PyBytes_AsString(lockspace), NULL /* event */, 0 /* flags */); Py_END_ALLOW_THREADS
if (fd < 0) { - __set_exception(fd, "Unable to register event fd"); + set_sanlock_error(fd, "Unable to register event fd"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -1545,11 +1545,11 @@ py_get_event(PyObject *self __unused, PyObject *args)
if (rv == -EAGAIN) break;
if (rv != 0) { - __set_exception(rv, "Unable to get events"); + set_sanlock_error(rv, "Unable to get events"); goto exit_fail; }
if ((item = PyDict_New()) == NULL) goto exit_fail; @@ -1636,11 +1636,11 @@ py_end_event(PyObject *self __unused, PyObject *args) Py_BEGIN_ALLOW_THREADS rv = sanlock_end_event(fd, PyBytes_AsString(lockspace), 0 /* flags */); Py_END_ALLOW_THREADS
if (rv < 0) { - __set_exception(rv, "Unable to unregister event fd"); + set_sanlock_error(rv, "Unable to unregister event fd"); goto finally; }
finally: Py_XDECREF(lockspace); @@ -1721,11 +1721,11 @@ py_set_event(PyObject *self __unused, PyObject *args, PyObject *keywds) Py_BEGIN_ALLOW_THREADS rv = sanlock_set_event(PyBytes_AsString(lockspace), &he, flags); Py_END_ALLOW_THREADS
if (rv < 0) { - __set_exception(rv, "Unable to set event"); + set_sanlock_error(rv, "Unable to set event"); goto finally; }
finally: Py_XDECREF(lockspace);
Underscore prefix is reserved for the compiler and standard library so we should not use it for our helpers.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 42b46fb..4155b51 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -20,11 +20,11 @@
#define BIND_ERROR -1000
/* Functions prototypes */ static void set_sanlock_error(int en, char *msg); -static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret); +static int parse_disks(PyObject *obj, struct sanlk_resource **res_ret); static void set_error(PyObject *exception, const char* format, PyObject* obj);
/* Sanlock module */ PyDoc_STRVAR(pydoc_sanlock, "\ Copyright (C) 2010-2011 Red Hat, Inc. All rights reserved.\n\ @@ -171,11 +171,11 @@ finally: Py_XDECREF(path); return rv; }
static int -__parse_resource(PyObject *obj, struct sanlk_resource **res_ret) +parse_disks(PyObject *obj, struct sanlk_resource **res_ret) { int i, num_disks, res_len; struct sanlk_resource *res;
num_disks = PyList_Size(obj); @@ -479,11 +479,11 @@ py_init_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) &PyList_Type, &disks, &max_hosts, &num_hosts, &use_aio)) { goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &res) < 0) { + if (parse_disks(disks, &res) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); @@ -745,11 +745,11 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) &PyList_Type, &disks, &max_hosts, &num_hosts, &clear, &align, §or)) { goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &rs) < 0) { + if (parse_disks(disks, &rs) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(rs->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); @@ -1116,11 +1116,11 @@ py_acquire(PyObject *self __unused, PyObject *args, PyObject *keywds) set_sanlock_error(EINVAL, "Invalid slkfd and pid values"); goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &res) < 0) { + if (parse_disks(disks, &res) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); @@ -1183,11 +1183,11 @@ py_release(PyObject *self __unused, PyObject *args, PyObject *keywds) &PyList_Type, &disks, &sanlockfd, &pid)) { goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &res) < 0) { + if (parse_disks(disks, &res) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); @@ -1239,11 +1239,11 @@ py_request(PyObject *self __unused, PyObject *args, PyObject *keywds) &PyList_Type, &disks, &action, &version)) { goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &res) < 0) { + if (parse_disks(disks, &res) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN); @@ -1311,11 +1311,11 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd &PyList_Type, &disks, &align, §or)) { goto finally; }
/* parse and check sanlock resource */ - if (__parse_resource(disks, &res) < 0) { + if (parse_disks(disks, &res) < 0) { goto finally; }
/* prepare sanlock names */ strncpy(res->lockspace_name, PyBytes_AsString(lockspace), SANLK_NAME_LEN);
Underscore prefix is reserved for the compiler and standard library so we should not use it for our helpers.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 4155b51..c44fee7 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -267,11 +267,11 @@ set_error(PyObject* exception, const char* format, PyObject* obj) PyErr_Format(exception, format, str_rep); Py_XDECREF(rep); }
static PyObject * -__hosts_to_list(struct sanlk_host *hss, int hss_count) +hosts_to_list(struct sanlk_host *hss, int hss_count) { int i; PyObject *ls_list = NULL, *ls_entry = NULL;
/* prepare the dictionary holding the information */ @@ -1071,11 +1071,11 @@ py_get_hosts(PyObject *self __unused, PyObject *args, PyObject *keywds) if (rv < 0) { set_sanlock_error(rv, "Sanlock get hosts failure"); goto finally; }
- ls_list = __hosts_to_list(hss, hss_count); + ls_list = hosts_to_list(hss, hss_count);
finally: Py_XDECREF(lockspace); free(hss); if (rv < 0) @@ -1337,11 +1337,11 @@ py_read_resource_owners(PyObject *self __unused, PyObject *args, PyObject *keywd if (rv != 0) { set_sanlock_error(rv, "Unable to read resource owners"); goto finally; }
- ls_list = __hosts_to_list(hss, hss_count); + ls_list = hosts_to_list(hss, hss_count);
finally: Py_XDECREF(lockspace); Py_XDECREF(resource); free(res);
Fix the years range in the python extension and python files license blob.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 4 ++-- python/setup.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index c44fee7..83ecf8a 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1,7 +1,7 @@ /* - * Copyright 2011 Red Hat, Inc. + * Copyright 2010-2019 Red Hat, Inc. * * This copyrighted material is made available to anyone wishing to use, * modify, copy, or redistribute it subject to the terms and conditions * of the GNU General Public License v2 or (at your option) any later version. */ @@ -25,11 +25,11 @@ static void set_sanlock_error(int en, char *msg); static int parse_disks(PyObject *obj, struct sanlk_resource **res_ret); static void set_error(PyObject *exception, const char* format, PyObject* obj);
/* Sanlock module */ PyDoc_STRVAR(pydoc_sanlock, "\ -Copyright (C) 2010-2011 Red Hat, Inc. All rights reserved.\n\ +Copyright (C) 2010-2019 Red Hat, Inc. All rights reserved.\n\ This copyrighted material is made available to anyone wishing to use,\n\ modify, copy, or redistribute it subject to the terms and conditions\n\ of the GNU General Public License v2 or (at your option) any later version.");
/* Sanlock exception */ diff --git a/python/setup.py b/python/setup.py index 91311a8..369451c 100644 --- a/python/setup.py +++ b/python/setup.py @@ -1,6 +1,6 @@ -# Copyright 2010-2011 Red Hat, Inc. +# Copyright 2010-2019 Red Hat, Inc. # # This copyrighted material is made available to anyone wishing to use, # modify, copy, or redistribute it subject to the terms and conditions # of the GNU General Public License v.2.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/example.py | 7 +++++++ tests/conftest.py | 6 ++++++ tests/constants.py | 6 ++++++ tests/daemon_test.py | 6 ++++++ tests/direct_test.py | 6 ++++++ tests/python_test.py | 6 ++++++ tests/storage.py | 6 ++++++ tests/units.py | 6 ++++++ tests/util.py | 6 ++++++ 9 files changed, 55 insertions(+)
diff --git a/python/example.py b/python/example.py index 6ef9f7d..370b17d 100644 --- a/python/example.py +++ b/python/example.py @@ -1,5 +1,12 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. + import pwd import grp import os import time import signal diff --git a/tests/conftest.py b/tests/conftest.py index 70e83d4..9dad677 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Fixtures for sanlock testing. """ from __future__ import absolute_import
diff --git a/tests/constants.py b/tests/constants.py index a7eb16b..4f45d13 100644 --- a/tests/constants.py +++ b/tests/constants.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Constants copied from sanlock source. """
# src/leader.h diff --git a/tests/daemon_test.py b/tests/daemon_test.py index 36648e3..26c64af 100644 --- a/tests/daemon_test.py +++ b/tests/daemon_test.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Test sanlock client operations. """ from __future__ import absolute_import
diff --git a/tests/direct_test.py b/tests/direct_test.py index a157bc4..b2cf2f1 100644 --- a/tests/direct_test.py +++ b/tests/direct_test.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Test sanlock direct options. """ from __future__ import absolute_import
diff --git a/tests/python_test.py b/tests/python_test.py index 3242f57..4320dad 100644 --- a/tests/python_test.py +++ b/tests/python_test.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Test sanlock python binding with sanlock daemon. """ from __future__ import absolute_import
diff --git a/tests/storage.py b/tests/storage.py index b7fcb2a..0fe6f56 100644 --- a/tests/storage.py +++ b/tests/storage.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ storage - provide storage for sanlock tests. """
import argparse diff --git a/tests/units.py b/tests/units.py index b47d668..166bff5 100644 --- a/tests/units.py +++ b/tests/units.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Constants for file/disk sizes. """
KiB = 1024 diff --git a/tests/util.py b/tests/util.py index 08db236..cce12b3 100644 --- a/tests/util.py +++ b/tests/util.py @@ -1,5 +1,11 @@ +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. """ Testing utilities """ from __future__ import absolute_import
The python extension source does not use this pointless text. Unify the module docstring to use the same license blob of its source.
For more info why "All rights reserved" is meaningless, see: https://github.com/libguestfs/nbdkit/commit/952ffe0fc7685ea775ed8879bf0b2cb9...
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 83ecf8a..2216322 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -25,11 +25,11 @@ static void set_sanlock_error(int en, char *msg); static int parse_disks(PyObject *obj, struct sanlk_resource **res_ret); static void set_error(PyObject *exception, const char* format, PyObject* obj);
/* Sanlock module */ PyDoc_STRVAR(pydoc_sanlock, "\ -Copyright (C) 2010-2019 Red Hat, Inc. All rights reserved.\n\ +Copyright (C) 2010-2019 Red Hat, Inc.\n\ This copyrighted material is made available to anyone wishing to use,\n\ modify, copy, or redistribute it subject to the terms and conditions\n\ of the GNU General Public License v2 or (at your option) any later version.");
/* Sanlock exception */
Using C99 enables useful features[1] is old enough so it should not be a portability issue.
[1] https://fedoramagazine.org/c99-features-gcc-fedora/
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/setup.py | 1 + 1 file changed, 1 insertion(+)
diff --git a/python/setup.py b/python/setup.py index 369451c..0f3d683 100644 --- a/python/setup.py +++ b/python/setup.py @@ -9,10 +9,11 @@ from distutils.core import setup, Extension sanlocklib = ['sanlock'] sanlock = Extension(name='sanlock', sources=['sanlock.c'], include_dirs=['../src'], library_dirs=['../src'], + extra_compile_args=["-std=c99"], extra_link_args=['-fPIE', '-Wl,-z,relro,-z,now'], libraries=sanlocklib)
version = None with open('../VERSION') as f:
Replace:
int i; for (i = 0; ...)
With:
for (int i = 0; ...)
This removes unneeded variable from the function, and limit the scope of the counter to the loop.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 2216322..ed9de0f 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -173,11 +173,11 @@ finally: }
static int parse_disks(PyObject *obj, struct sanlk_resource **res_ret) { - int i, num_disks, res_len; + int num_disks, res_len; struct sanlk_resource *res;
num_disks = PyList_Size(obj);
res_len = sizeof(struct sanlk_resource) + @@ -190,11 +190,11 @@ parse_disks(PyObject *obj, struct sanlk_resource **res_ret) }
memset(res, 0, res_len); res->num_disks = num_disks;
- for (i = 0; i < num_disks; i++) { + for (int i = 0; i < num_disks; i++) { PyObject *disk = PyList_GetItem(obj,i);
if (!parse_single_disk(disk, &(res->disks[i]))) { goto exit_fail; } @@ -269,18 +269,17 @@ set_error(PyObject* exception, const char* format, PyObject* obj) }
static PyObject * hosts_to_list(struct sanlk_host *hss, int hss_count) { - int i; PyObject *ls_list = NULL, *ls_entry = NULL;
/* prepare the dictionary holding the information */ if ((ls_list = PyList_New(0)) == NULL) goto exit_fail;
- for (i = 0; i < hss_count; i++) { + for (int i = 0; i < hss_count; i++) {
/* fill the dictionary information */ ls_entry = Py_BuildValue( "{s:K,s:K,s:K,s:I,s:I}", "host_id", hss[i].host_id, @@ -979,11 +978,11 @@ process of being acquired, and LSFLAG_REM if it's in the process of being\n\ released.\n");
static PyObject * py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds) { - int rv, i, lss_count; + int rv, lss_count; struct sanlk_lockspace *lss = NULL; PyObject *ls_list = NULL, *ls_entry = NULL;
/* get all the lockspaces (gil disabled) */ Py_BEGIN_ALLOW_THREADS @@ -997,11 +996,11 @@ py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds)
/* prepare the dictionary holding the information */ if ((ls_list = PyList_New(0)) == NULL) goto exit_fail;
- for (i = 0; i < lss_count; i++) { + for (int i = 0; i < lss_count; i++) {
/* fill the dictionary information */ ls_entry = Py_BuildValue( #if PY_MAJOR_VERSION == 2 "{s:s,s:K,s:s,s:K,s:I}", @@ -1352,12 +1351,12 @@ finally: }
static int parse_killpath_item(PyObject *item, char *kpargs, size_t *kplen) { - int rv = 0, i; - size_t arg_len; + int rv = 0; + size_t arg_len = 0; PyObject *path = NULL; const char *p = NULL;
if (!pypath_converter(item, &path)) { goto finally; @@ -1365,11 +1364,11 @@ parse_killpath_item(PyObject *item, char *kpargs, size_t *kplen) 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++) { + for (int i = 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) { @@ -1405,12 +1404,12 @@ 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 = -1, i, num_args, sanlockfd = -1; - size_t kplen; + int rv = -1, num_args, sanlockfd = -1; + size_t kplen = 0; char kpargs[SANLK_HELPER_ARGS_LEN]; PyObject *path = NULL; PyObject *argslist;
static char *kwlist[] = {"path", "args", "slkfd", NULL}; @@ -1429,11 +1428,11 @@ 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 (i = 0, kplen = 0; i < num_args; i++) { + for (int i = 0; i < num_args; i++) { PyObject *item = PyList_GetItem(argslist, i); if (!parse_killpath_item(item, kpargs, &kplen)) { goto finally; } }
When calling free() we don't need to check if a point is NULL.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/sanlock.c b/python/sanlock.c index ed9de0f..4c3e0a0 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -1025,11 +1025,11 @@ py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds) free(lss); return ls_list;
/* failure */ exit_fail: - if (lss) free(lss); + free(lss); Py_XDECREF(ls_entry); Py_XDECREF(ls_list); return NULL; }
This function used error prone style, using local loop variable in the scope of the function, making cleanup more risky.
Move ls_entry info the loop scope, and use PyList_SetItem(), stealing the reference to ls_entry, so we need to decrease the reference count only on errors.
To use PyList_SetItem() we need to allocate the entire list upfront, which is good idea anyway.
While refactoring, remove some useless and wrong comments.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 4c3e0a0..4f80379 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -269,40 +269,35 @@ set_error(PyObject* exception, const char* format, PyObject* obj) }
static PyObject * hosts_to_list(struct sanlk_host *hss, int hss_count) { - PyObject *ls_list = NULL, *ls_entry = NULL; - - /* prepare the dictionary holding the information */ - if ((ls_list = PyList_New(0)) == NULL) + PyObject *ls_list = PyList_New(hss_count); + if (ls_list == NULL) goto exit_fail;
for (int i = 0; i < hss_count; i++) { - - /* fill the dictionary information */ - ls_entry = Py_BuildValue( - "{s:K,s:K,s:K,s:I,s:I}", - "host_id", hss[i].host_id, - "generation", hss[i].generation, - "timestamp", hss[i].timestamp, - "io_timeout", hss[i].io_timeout, - "flags", hss[i].flags); + PyObject *ls_entry = Py_BuildValue( + "{s:K,s:K,s:K,s:I,s:I}", + "host_id", hss[i].host_id, + "generation", hss[i].generation, + "timestamp", hss[i].timestamp, + "io_timeout", hss[i].io_timeout, + "flags", hss[i].flags); if (ls_entry == NULL) goto exit_fail;
- if (PyList_Append(ls_list, ls_entry) != 0) + /* Steals reference to ls_entry. */ + if (PyList_SetItem(ls_list, i, ls_entry) != 0) { + Py_DECREF(ls_entry); goto exit_fail; - - Py_DECREF(ls_entry); + } }
return ls_list;
- /* failure */ exit_fail: - Py_XDECREF(ls_entry); Py_XDECREF(ls_list); return NULL; }
/* register */
py_get_lockspaces() was using the same error prone error cleanup as hosts_to_list(), mixed with code to call sanlock, to make the code harder to follow.
Extract lockspaces_to_list() helper, implemented exactly like hosts_to_list. This simplify py_get_lockspaces() and allow using the simpler finally pattern we used in other functions.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 76 +++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 36 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 4f80379..556627c 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -961,10 +961,46 @@ finally: if (rv != 0) return NULL; Py_RETURN_NONE; }
+static PyObject * +lockspaces_to_list(struct sanlk_lockspace *lss, int lss_count) +{ + PyObject *ls_list = PyList_New(lss_count); + if (ls_list == NULL) + goto exit_fail; + + for (int i = 0; i < lss_count; i++) { + PyObject *ls_entry = Py_BuildValue( +#if PY_MAJOR_VERSION == 2 + "{s:s,s:K,s:s,s:K,s:I}", +#else + "{s:y,s:K,s:s,s:K,s:I}", +#endif + "lockspace", lss[i].name, + "host_id", lss[i].host_id, + "path", lss[i].host_id_disk.path, + "offset", lss[i].host_id_disk.offset, + "flags", lss[i].flags); + if (ls_entry == NULL) + goto exit_fail; + + /* Steals reference to ls_entry. */ + if (PyList_SetItem(ls_list, i, ls_entry) != 0) { + Py_DECREF(ls_entry); + goto exit_fail; + } + } + + return ls_list; + +exit_fail: + Py_XDECREF(ls_list); + return NULL; +} + /* get_lockspaces */ PyDoc_STRVAR(pydoc_get_lockspaces, "\ get_lockspaces() -> list\n\ Return the list of lockspaces currently managed by sanlock. The reported\n\ flag indicates whether the lockspace is acquired (0) or in transition.\n\ @@ -975,59 +1011,27 @@ released.\n"); static PyObject * py_get_lockspaces(PyObject *self __unused, PyObject *args, PyObject *keywds) { int rv, lss_count; struct sanlk_lockspace *lss = NULL; - PyObject *ls_list = NULL, *ls_entry = NULL; + PyObject *ls_list = NULL;
/* get all the lockspaces (gil disabled) */ Py_BEGIN_ALLOW_THREADS rv = sanlock_get_lockspaces(&lss, &lss_count, 0); Py_END_ALLOW_THREADS
if (rv < 0) { set_sanlock_error(rv, "Sanlock get lockspaces failure"); - goto exit_fail; + goto finally; }
- /* prepare the dictionary holding the information */ - if ((ls_list = PyList_New(0)) == NULL) - goto exit_fail; + ls_list = lockspaces_to_list(lss, lss_count);
- for (int i = 0; i < lss_count; i++) { - - /* fill the dictionary information */ - ls_entry = Py_BuildValue( -#if PY_MAJOR_VERSION == 2 - "{s:s,s:K,s:s,s:K,s:I}", -#else - "{s:y,s:K,s:s,s:K,s:I}", -#endif - "lockspace", lss[i].name, - "host_id", lss[i].host_id, - "path", lss[i].host_id_disk.path, - "offset", lss[i].host_id_disk.offset, - "flags", lss[i].flags); - if (ls_entry == NULL) - goto exit_fail; - - if (PyList_Append(ls_list, ls_entry) != 0) - goto exit_fail; - - Py_DECREF(ls_entry); - } - - /* success */ +finally: free(lss); return ls_list; - - /* failure */ -exit_fail: - free(lss); - Py_XDECREF(ls_entry); - Py_XDECREF(ls_list); - return NULL; }
/* get_hosts */ PyDoc_STRVAR(pydoc_get_hosts, "\ get_hosts(lockspace, host_id=0) -> list\n\
Some tabs sneaked into the module before we added the vim modeline. Some Py_BuildValue calls used too much indentation.
Signed-off-by: Nir Soffer nsoffer@redhat.com --- python/sanlock.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/python/sanlock.c b/python/sanlock.c index 556627c..12e2195 100644 --- a/python/sanlock.c +++ b/python/sanlock.c @@ -214,17 +214,17 @@ static int add_sector_flag(int sector, uint32_t *flags) { switch (sector) { case SECTOR_SIZE_512: *flags |= SANLK_LSF_SECTOR512; - break; + break; case SECTOR_SIZE_4K: *flags |= SANLK_LSF_SECTOR4K; - break; + break; default: - PyErr_Format(PyExc_ValueError, "Invalid sector value: %d", sector); - return -1; + PyErr_Format(PyExc_ValueError, "Invalid sector value: %d", sector); + return -1; } return 0; }
enum { @@ -238,23 +238,23 @@ static int add_align_flag(long align, uint32_t *flags) { switch (align) { case ALIGNMENT_1M: *flags |= SANLK_RES_ALIGN1M; - break; + break; case ALIGNMENT_2M: *flags |= SANLK_RES_ALIGN2M; - break; + break; case ALIGNMENT_4M: *flags |= SANLK_RES_ALIGN4M; - break; + break; case ALIGNMENT_8M: *flags |= SANLK_RES_ALIGN8M; - break; + break; default: - PyErr_Format(PyExc_ValueError, "Invalid align value: %ld", align); - return -1; + PyErr_Format(PyExc_ValueError, "Invalid align value: %ld", align); + return -1; } return 0; }
static void @@ -611,16 +611,16 @@ py_read_lockspace(PyObject *self __unused, PyObject *args, PyObject *keywds) }
/* fill the information dictionary */ ls_info = Py_BuildValue( #if PY_MAJOR_VERSION == 2 - "{s:s,s:I}", + "{s:s,s:I}", #else - "{s:y,s:I}", + "{s:y,s:I}", #endif - "lockspace", ls.name, - "iotimeout", io_timeout); + "lockspace", ls.name, + "iotimeout", io_timeout); if (ls_info == NULL) goto finally;
finally: Py_XDECREF(path);
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@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 @@ -637,78 +637,78 @@ 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 = -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) { set_sanlock_error(rv, "Sanlock resource read failure"); goto finally; }
/* 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 */ PyDoc_STRVAR(pydoc_write_resource, "\ write_resource(lockspace, resource, disks, max_hosts=0, num_hosts=0, \ @@ -724,11 +724,11 @@ static PyObject * 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;
static char *kwlist[] = {"lockspace", "resource", "disks", "max_hosts", "num_hosts", "clear", "align", "sector", NULL}; @@ -739,43 +739,43 @@ py_write_resource(PyObject *self __unused, PyObject *args, PyObject *keywds) &PyList_Type, &disks, &max_hosts, &num_hosts, &clear, &align, §or)) { goto finally; }
/* 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) { flags |= SANLK_WRITE_CLEAR; }
/* 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) { set_sanlock_error(rv, "Sanlock resource write failure"); goto finally; }
finally: Py_XDECREF(lockspace); Py_XDECREF(resource); - free(rs); + free(res); if (rv != 0) return NULL; Py_RETURN_NONE; }
Looks good to me
Lately I spent lot of time reviewing changes to the this messy code, and it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310 ++++++++++++++++++++----------------------- python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
-- 2.17.2 _______________________________________________ sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost ed.org
On Mon, Jun 17, 2019 at 3:30 PM Vojtech Juranek vjuranek@redhat.com wrote:
Looks good to me
Thanks for reviewing.
Lets wait for Amit since he has more experience with this code.
Lately I spent lot of time reviewing changes to the this messy code, and
it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310 ++++++++++++++++++++----------------------- python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
-- 2.17.2 _______________________________________________ sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to
sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives:
https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost
ed.org
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahoste...
On Mon, Jun 17, 2019 at 5:00 PM Nir Soffer nsoffer@redhat.com wrote:
On Mon, Jun 17, 2019 at 3:30 PM Vojtech Juranek vjuranek@redhat.com wrote:
Looks good to me
Thanks for reviewing.
Lets wait for Amit since he has more experience with this code.
Amit, did you have time to look at these patches?
Lately I spent lot of time reviewing changes to the this messy code, and
it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310 ++++++++++++++++++++----------------------- python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
-- 2.17.2 _______________________________________________ sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to
sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives:
https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost
ed.org
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahoste...
LGTM, as long as the legal department is in peace with the removed copyright code comment .
On Tue, Jun 25, 2019 at 12:58 AM Nir Soffer nsoffer@redhat.com wrote:
On Mon, Jun 17, 2019 at 5:00 PM Nir Soffer nsoffer@redhat.com wrote:
On Mon, Jun 17, 2019 at 3:30 PM Vojtech Juranek vjuranek@redhat.com wrote:
Looks good to me
Thanks for reviewing.
Lets wait for Amit since he has more experience with this code.
Amit, did you have time to look at these patches?
Lately I spent lot of time reviewing changes to the this messy code, and
it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310 ++++++++++++++++++++----------------------- python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
-- 2.17.2 _______________________________________________ sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to
sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost
ed.org
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahoste...
Cool, thanks!
On Tue, Jun 25, 2019 at 2:43 PM Amit Bawer abawer@redhat.com wrote:
LGTM, as long as the legal department is in peace with the removed copyright code comment .
On Tue, Jun 25, 2019 at 12:58 AM Nir Soffer nsoffer@redhat.com wrote:
On Mon, Jun 17, 2019 at 5:00 PM Nir Soffer nsoffer@redhat.com wrote:
On Mon, Jun 17, 2019 at 3:30 PM Vojtech Juranek vjuranek@redhat.com wrote:
Looks good to me
Thanks for reviewing.
Lets wait for Amit since he has more experience with this code.
Amit, did you have time to look at these patches?
Lately I spent lot of time reviewing changes to the this messy code, and
it is time to clean up the mess. I'm planning many more cleanups, but this is enough for today.
Nir Soffer (14): python: Remove CPyChecker support python: Rename __set_exception to set_sanlock_error python: Rename __parse_resource to parse_disks python: Rename __hosts_to_list to hosts_to_list python: Fix copyright years python: Add missing license blob to python files python: Remove meaningless "All rights reserved" python: Use C99 standard python: Limit the scope of for loop counter python: Remove unneeded check for null python: Streamline hosts_to_list python: Streamline py_get_lockspaces() python: Unify indentation python: Unify resource variables names
python/example.py | 7 + python/sanlock.c | 310
++++++++++++++++++++-----------------------
python/setup.py | 3 +- tests/conftest.py | 6 + tests/constants.py | 6 + tests/daemon_test.py | 6 + tests/direct_test.py | 6 + tests/python_test.py | 6 + tests/storage.py | 6 + tests/units.py | 6 + tests/util.py | 6 + 11 files changed, 204 insertions(+), 164 deletions(-)
-- 2.17.2 _______________________________________________ sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to
sanlock-devel-leave@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahost
ed.org
sanlock-devel mailing list -- sanlock-devel@lists.fedorahosted.org To unsubscribe send an email to sanlock-devel-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sanlock-devel@lists.fedorahoste...
sanlock-devel@lists.fedorahosted.org
-
Amit Bawer -
Nir Soffer -
Nir Soffer -
Vojtech Juranek