On Sun, Jun 16, 2019 at 11:50 PM Nir Soffer <nirsof@gmail.com> wrote:Replace duplicate code for allocating and initializing sanlk_resource
struct with a helper.
Signed-off-by: Nir Soffer <nsoffer@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
@@ -170,29 +170,38 @@ parse_single_disk(PyObject* disk, struct sanlk_disk* res_disk)
finally:
Py_XDECREF(path);
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;What about declaring the local variables when first used, thus immediately initializing them? I think C99 provides that.
I.e.,int num_disks = PyList_Size(obj);struct sanlk_resource *res = create_resource(num_disks);Moreover, in this case they will be at the beginning of the block anyway.
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);
if (!parse_single_disk(disk, &(res->disks[i]))) {
@@ -637,30 +646,21 @@ 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, res_len, sector = SECTOR_SIZE_512;
+ int rv = -1, sector = SECTOR_SIZE_512;
long align = ALIGNMENT_1M;
PyObject *path = 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 */
- 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 */);Not sure the comment is needed.+ 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,
pypath_converter, &path, &(res->disks[0].offset), &align, §or)) {
goto finally;
--
2.17.2