New subject: [PATCH v2 4/8] python: Extract create_resource() helper

17 Jun 2019

On Mon, Jun 17, 2019 at 1:04 PM Pavel Bar pbar@redhat.com wrote:
...
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.
This is not needed to fix the problem I'm trying to solve here.
We can do this work later if needed.
...
...
 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,


&sector)) {
         goto finally;
--
2.17.2

    

Re: [PATCH v2 4/8] python: Extract create_resource() helper

Signed-off-by: Nir Soffer nsoffer@redhat.com

Signed-off-by: Nir Soffer nsoffer@redhat.com