2017-12-15 10:27:04

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 0/7] RapidIO: Adjustments for some function implementations

From: Markus Elfring <[email protected]>
Date: Fri, 15 Dec 2017 11:20:33 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (7):
Delete an error message for a failed memory allocation in rio_init_mports()
Adjust 12 checks for null pointers
Adjust five function calls together with a variable assignment
Improve a size determination in five functions
Delete an unnecessary variable initialisation in three functions
Return an error code only as a constant in two functions
Move 12 EXPORT_SYMBOL_GPL() calls to function implementations

drivers/rapidio/rio.c | 110 ++++++++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 62 deletions(-)

--
2.15.1


2017-12-15 10:29:33

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 1/7] rapidio: Delete an error message for a failed memory allocation in rio_init_mports()

From: Markus Elfring <[email protected]>
Date: Thu, 14 Dec 2017 15:11:12 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index 38d949405618..032ede23a8cb 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -2189,7 +2189,6 @@ int rio_init_mports(void)

work = kcalloc(n, sizeof *work, GFP_KERNEL);
if (!work) {
- pr_err("RIO: no memory for work struct\n");
destroy_workqueue(rio_wq);
goto no_disc;
}
--
2.15.1

2017-12-15 10:32:14

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 3/7] rapidio: Adjust five function calls together with a variable assignment

From: Markus Elfring <[email protected]>
Date: Thu, 14 Dec 2017 16:07:07 +0100

The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index f89085564c2c..b9dc932ce19e 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -252,9 +252,9 @@ int rio_request_inb_mbox(struct rio_mport *mport,
rio_init_mbox_res(res, mbox, mbox);

/* Make sure this mailbox isn't in use */
- if ((rc =
- request_resource(&mport->riores[RIO_INB_MBOX_RESOURCE],
- res)) < 0) {
+ rc = request_resource(&mport->riores[RIO_INB_MBOX_RESOURCE],
+ res);
+ if (rc < 0) {
kfree(res);
goto out;
}
@@ -335,9 +335,9 @@ int rio_request_outb_mbox(struct rio_mport *mport,
rio_init_mbox_res(res, mbox, mbox);

/* Make sure this outbound mailbox isn't in use */
- if ((rc =
- request_resource(&mport->riores[RIO_OUTB_MBOX_RESOURCE],
- res)) < 0) {
+ rc = request_resource(&mport->riores[RIO_OUTB_MBOX_RESOURCE],
+ res);
+ if (rc < 0) {
kfree(res);
goto out;
}
@@ -406,9 +406,9 @@ rio_setup_inb_dbell(struct rio_mport *mport, void *dev_id, struct resource *res,
u16 info))
{
int rc = 0;
- struct rio_dbell *dbell;
+ struct rio_dbell *dbell = kmalloc(sizeof(*dbell), GFP_KERNEL);

- if (!(dbell = kmalloc(sizeof(struct rio_dbell), GFP_KERNEL))) {
+ if (!dbell) {
rc = -ENOMEM;
goto out;
}
@@ -452,9 +452,9 @@ int rio_request_inb_dbell(struct rio_mport *mport,
rio_init_dbell_res(res, start, end);

/* Make sure these doorbells aren't in use */
- if ((rc =
- request_resource(&mport->riores[RIO_DOORBELL_RESOURCE],
- res)) < 0) {
+ rc = request_resource(&mport->riores[RIO_DOORBELL_RESOURCE],
+ res);
+ if (rc < 0) {
kfree(res);
goto out;
}
@@ -1411,7 +1411,9 @@ rio_mport_get_feature(struct rio_mport * port, int local, u16 destid,
ext_ftr_ptr, &ftr_header);
if (RIO_GET_BLOCK_ID(ftr_header) == ftr)
return ext_ftr_ptr;
- if (!(ext_ftr_ptr = RIO_GET_BLOCK_PTR(ftr_header)))
+
+ ext_ftr_ptr = RIO_GET_BLOCK_PTR(ftr_header);
+ if (!ext_ftr_ptr)
break;
}

--
2.15.1

2017-12-15 10:33:25

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 4/7] rapidio: Improve a size determination in five functions

From: Markus Elfring <[email protected]>
Date: Thu, 14 Dec 2017 16:24:51 +0100

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index b9dc932ce19e..90534365e46c 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -110,9 +110,8 @@ EXPORT_SYMBOL(rio_query_mport);
*/
struct rio_net *rio_alloc_net(struct rio_mport *mport)
{
- struct rio_net *net;
+ struct rio_net *net = kzalloc(sizeof(*net), GFP_KERNEL);

- net = kzalloc(sizeof(struct rio_net), GFP_KERNEL);
if (net) {
INIT_LIST_HEAD(&net->node);
INIT_LIST_HEAD(&net->devices);
@@ -246,8 +245,7 @@ int rio_request_inb_mbox(struct rio_mport *mport,
if (!mport->ops->open_inb_mbox)
goto out;

- res = kzalloc(sizeof(struct resource), GFP_KERNEL);
-
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
if (res) {
rio_init_mbox_res(res, mbox, mbox);

@@ -329,8 +327,7 @@ int rio_request_outb_mbox(struct rio_mport *mport,
if (!mport->ops->open_outb_mbox)
goto out;

- res = kzalloc(sizeof(struct resource), GFP_KERNEL);
-
+ res = kzalloc(sizeof(*res), GFP_KERNEL);
if (res) {
rio_init_mbox_res(res, mbox, mbox);

@@ -445,8 +442,7 @@ int rio_request_inb_dbell(struct rio_mport *mport,
u16 dst, u16 info))
{
int rc = 0;
-
- struct resource *res = kzalloc(sizeof(struct resource), GFP_KERNEL);
+ struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);

if (res) {
rio_init_dbell_res(res, start, end);
@@ -568,9 +564,8 @@ int rio_add_mport_pw_handler(struct rio_mport *mport, void *context,
void *context, union rio_pw_msg *msg, int step))
{
int rc = 0;
- struct rio_pwrite *pwrite;
+ struct rio_pwrite *pwrite = kzalloc(sizeof(*pwrite), GFP_KERNEL);

- pwrite = kzalloc(sizeof(struct rio_pwrite), GFP_KERNEL);
if (!pwrite) {
rc = -ENOMEM;
goto out;
--
2.15.1

2017-12-15 10:34:42

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 5/7] rapidio: Delete an unnecessary variable initialisation in three functions

From: Markus Elfring <[email protected]>
Date: Fri, 15 Dec 2017 10:21:15 +0100

The local variable "rc" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index 90534365e46c..4604410fb001 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -441,7 +441,7 @@ int rio_request_inb_dbell(struct rio_mport *mport,
void (*dinb) (struct rio_mport * mport, void *dev_id, u16 src,
u16 dst, u16 info))
{
- int rc = 0;
+ int rc;
struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);

if (res) {
@@ -693,7 +693,7 @@ EXPORT_SYMBOL_GPL(rio_pw_enable);
int rio_map_inb_region(struct rio_mport *mport, dma_addr_t local,
u64 rbase, u32 size, u32 rflags)
{
- int rc = 0;
+ int rc;
unsigned long flags;

if (!mport->ops->map_inb)
@@ -737,7 +737,7 @@ EXPORT_SYMBOL_GPL(rio_unmap_inb_region);
int rio_map_outb_region(struct rio_mport *mport, u16 destid, u64 rbase,
u32 size, u32 rflags, dma_addr_t *local)
{
- int rc = 0;
+ int rc;
unsigned long flags;

if (!mport->ops->map_outb)
--
2.15.1

2017-12-15 10:35:29

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 6/7] rapidio: Return an error code only as a constant in two functions

From: Markus Elfring <[email protected]>
Date: Fri, 15 Dec 2017 10:40:19 +0100

* Return an error code without storing it in an intermediate variable.

* Delete the label "out" and local variable "rc" which became unnecessary
with this refactoring.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index 4604410fb001..d095a18257c2 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -402,13 +402,10 @@ rio_setup_inb_dbell(struct rio_mport *mport, void *dev_id, struct resource *res,
void (*dinb) (struct rio_mport * mport, void *dev_id, u16 src, u16 dst,
u16 info))
{
- int rc = 0;
struct rio_dbell *dbell = kmalloc(sizeof(*dbell), GFP_KERNEL);

- if (!dbell) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!dbell)
+ return -ENOMEM;

dbell->res = res;
dbell->dinb = dinb;
@@ -417,9 +414,7 @@ rio_setup_inb_dbell(struct rio_mport *mport, void *dev_id, struct resource *res,
mutex_lock(&mport->lock);
list_add_tail(&dbell->node, &mport->dbells);
mutex_unlock(&mport->lock);
-
- out:
- return rc;
+ return 0;
}

/**
@@ -563,21 +558,17 @@ int rio_add_mport_pw_handler(struct rio_mport *mport, void *context,
int (*pwcback)(struct rio_mport *mport,
void *context, union rio_pw_msg *msg, int step))
{
- int rc = 0;
struct rio_pwrite *pwrite = kzalloc(sizeof(*pwrite), GFP_KERNEL);

- if (!pwrite) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!pwrite)
+ return -ENOMEM;

pwrite->pwcback = pwcback;
pwrite->context = context;
mutex_lock(&mport->lock);
list_add_tail(&pwrite->node, &mport->pwrites);
mutex_unlock(&mport->lock);
-out:
- return rc;
+ return 0;
}
EXPORT_SYMBOL_GPL(rio_add_mport_pw_handler);

--
2.15.1

2017-12-15 10:35:48

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 2/7] rapidio: Adjust 12 checks for null pointers

From: Markus Elfring <[email protected]>
Date: Thu, 14 Dec 2017 15:27:07 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index 032ede23a8cb..f89085564c2c 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -243,7 +243,7 @@ int rio_request_inb_mbox(struct rio_mport *mport,
int rc = -ENOSYS;
struct resource *res;

- if (mport->ops->open_inb_mbox == NULL)
+ if (!mport->ops->open_inb_mbox)
goto out;

res = kzalloc(sizeof(struct resource), GFP_KERNEL);
@@ -326,7 +326,7 @@ int rio_request_outb_mbox(struct rio_mport *mport,
int rc = -ENOSYS;
struct resource *res;

- if (mport->ops->open_outb_mbox == NULL)
+ if (!mport->ops->open_outb_mbox)
goto out;

res = kzalloc(sizeof(struct resource), GFP_KERNEL);
@@ -632,7 +632,7 @@ int rio_request_inb_pwrite(struct rio_dev *rdev,
int rc = 0;

spin_lock(&rio_global_list_lock);
- if (rdev->pwcback != NULL)
+ if (rdev->pwcback)
rc = -ENOMEM;
else
rdev->pwcback = pwcback;
@@ -975,7 +975,7 @@ rio_chk_dev_route(struct rio_dev *rdev, struct rio_dev **nrdev, int *npnum)
rdev = rdev->prev;
}

- if (prev == NULL)
+ if (!prev)
goto err_out;

p_port = prev->rswitch->route_table[rdev->destid];
@@ -1054,7 +1054,7 @@ rio_get_input_status(struct rio_dev *rdev, int pnum, u32 *lnkresp)
RIO_MNT_REQ_CMD_IS);

/* Exit if the response is not expected */
- if (lnkresp == NULL)
+ if (!lnkresp)
return 0;

checkcount = 3;
@@ -1696,7 +1696,7 @@ int rio_route_add_entry(struct rio_dev *rdev,

spin_lock(&rdev->rswitch->lock);

- if (ops == NULL || ops->add_entry == NULL) {
+ if (!ops || !ops->add_entry) {
rc = rio_std_route_add_entry(rdev->net->hport, rdev->destid,
rdev->hopcount, table,
route_destid, route_port);
@@ -1749,7 +1749,7 @@ int rio_route_get_entry(struct rio_dev *rdev, u16 table,

spin_lock(&rdev->rswitch->lock);

- if (ops == NULL || ops->get_entry == NULL) {
+ if (!ops || !ops->get_entry) {
rc = rio_std_route_get_entry(rdev->net->hport, rdev->destid,
rdev->hopcount, table,
route_destid, route_port);
@@ -1797,7 +1797,7 @@ int rio_route_clr_table(struct rio_dev *rdev, u16 table, int lock)

spin_lock(&rdev->rswitch->lock);

- if (ops == NULL || ops->clr_table == NULL) {
+ if (!ops || !ops->clr_table) {
rc = rio_std_route_clr_table(rdev->net->hport, rdev->destid,
rdev->hopcount, table);
} else if (try_module_get(ops->owner)) {
@@ -1889,7 +1889,7 @@ struct dma_async_tx_descriptor *rio_dma_prep_xfer(struct dma_chan *dchan,
{
struct rio_dma_ext rio_ext;

- if (dchan->device->device_prep_slave_sg == NULL) {
+ if (!dchan->device->device_prep_slave_sg) {
pr_err("%s: prep_rio_sg == NULL\n", __func__);
return NULL;
}
--
2.15.1

2017-12-15 10:36:38

by SF Markus Elfring

[permalink] [raw]
Subject: [PATCH 7/7] rapidio: Move 12 EXPORT_SYMBOL_GPL() calls to function implementations

From: Markus Elfring <[email protected]>
Date: Fri, 15 Dec 2017 11:08:31 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <[email protected]>
---
drivers/rapidio/rio.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
index d095a18257c2..83406696c7aa 100644
--- a/drivers/rapidio/rio.c
+++ b/drivers/rapidio/rio.c
@@ -81,6 +81,7 @@ u16 rio_local_get_device_id(struct rio_mport *port)

return (RIO_GET_DID(port->sys_size, result));
}
+EXPORT_SYMBOL_GPL(rio_local_get_device_id);

/**
* rio_query_mport - Query mport device attributes
@@ -275,6 +276,7 @@ int rio_request_inb_mbox(struct rio_mport *mport,
out:
return rc;
}
+EXPORT_SYMBOL_GPL(rio_request_inb_mbox);

/**
* rio_release_inb_mbox - release inbound mailbox message service
@@ -303,6 +305,7 @@ int rio_release_inb_mbox(struct rio_mport *mport, int mbox)

return 0;
}
+EXPORT_SYMBOL_GPL(rio_release_inb_mbox);

/**
* rio_request_outb_mbox - request outbound mailbox service
@@ -357,6 +360,7 @@ int rio_request_outb_mbox(struct rio_mport *mport,
out:
return rc;
}
+EXPORT_SYMBOL_GPL(rio_request_outb_mbox);

/**
* rio_release_outb_mbox - release outbound mailbox message service
@@ -385,6 +389,7 @@ int rio_release_outb_mbox(struct rio_mport *mport, int mbox)

return 0;
}
+EXPORT_SYMBOL_GPL(rio_release_outb_mbox);

/**
* rio_setup_inb_dbell - bind inbound doorbell callback
@@ -458,6 +463,7 @@ int rio_request_inb_dbell(struct rio_mport *mport,
out:
return rc;
}
+EXPORT_SYMBOL_GPL(rio_request_inb_dbell);

/**
* rio_release_inb_dbell - release inbound doorbell message service
@@ -499,6 +505,7 @@ int rio_release_inb_dbell(struct rio_mport *mport, u16 start, u16 end)
out:
return rc;
}
+EXPORT_SYMBOL_GPL(rio_release_inb_dbell);

/**
* rio_request_outb_dbell - request outbound doorbell message range
@@ -527,6 +534,7 @@ struct resource *rio_request_outb_dbell(struct rio_dev *rdev, u16 start,

return res;
}
+EXPORT_SYMBOL_GPL(rio_request_outb_dbell);

/**
* rio_release_outb_dbell - release outbound doorbell message range
@@ -544,6 +552,7 @@ int rio_release_outb_dbell(struct rio_dev *rdev, struct resource *res)

return rc;
}
+EXPORT_SYMBOL_GPL(rio_release_outb_dbell);

/**
* rio_add_mport_pw_handler - add port-write message handler into the list
@@ -1450,6 +1459,7 @@ struct rio_dev *rio_get_asm(u16 vid, u16 did,
spin_unlock(&rio_global_list_lock);
return rdev;
}
+EXPORT_SYMBOL_GPL(rio_get_asm);

/**
* rio_get_device - Begin or continue searching for a RIO device by vid/did
@@ -1469,6 +1479,7 @@ struct rio_dev *rio_get_device(u16 vid, u16 did, struct rio_dev *from)
{
return rio_get_asm(vid, did, RIO_ANY_ID, RIO_ANY_ID, from);
}
+EXPORT_SYMBOL_GPL(rio_get_device);

/**
* rio_std_route_add_entry - Add switch route table entry using standard
@@ -2203,6 +2214,7 @@ int rio_init_mports(void)

return 0;
}
+EXPORT_SYMBOL_GPL(rio_init_mports);

static int rio_get_hdid(int index)
{
@@ -2317,16 +2329,3 @@ int rio_unregister_mport(struct rio_mport *port)
return 0;
}
EXPORT_SYMBOL_GPL(rio_unregister_mport);
-
-EXPORT_SYMBOL_GPL(rio_local_get_device_id);
-EXPORT_SYMBOL_GPL(rio_get_device);
-EXPORT_SYMBOL_GPL(rio_get_asm);
-EXPORT_SYMBOL_GPL(rio_request_inb_dbell);
-EXPORT_SYMBOL_GPL(rio_release_inb_dbell);
-EXPORT_SYMBOL_GPL(rio_request_outb_dbell);
-EXPORT_SYMBOL_GPL(rio_release_outb_dbell);
-EXPORT_SYMBOL_GPL(rio_request_inb_mbox);
-EXPORT_SYMBOL_GPL(rio_release_inb_mbox);
-EXPORT_SYMBOL_GPL(rio_request_outb_mbox);
-EXPORT_SYMBOL_GPL(rio_release_outb_mbox);
-EXPORT_SYMBOL_GPL(rio_init_mports);
--
2.15.1

2017-12-21 14:52:20

by Bounine, Alexandre

[permalink] [raw]
Subject: RE: [PATCH 0/7] RapidIO: Adjustments for some function implementations

Acked-by: Alexandre Bounine <[email protected]>

-----Original Message-----
From: SF Markus Elfring [mailto:[email protected]]
Sent: Friday, December 15, 2017 5:27 AM
To: [email protected]; Bounine, Alexandre <[email protected]>; Matt Porter <[email protected]>
Cc: LKML <[email protected]>
Subject: [PATCH 0/7] RapidIO: Adjustments for some function implementations

From: Markus Elfring <[email protected]>
Date: Fri, 15 Dec 2017 11:20:33 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (7):
Delete an error message for a failed memory allocation in rio_init_mports()
Adjust 12 checks for null pointers
Adjust five function calls together with a variable assignment
Improve a size determination in five functions
Delete an unnecessary variable initialisation in three functions
Return an error code only as a constant in two functions
Move 12 EXPORT_SYMBOL_GPL() calls to function implementations

drivers/rapidio/rio.c | 110 ++++++++++++++++++++++----------------------------
1 file changed, 48 insertions(+), 62 deletions(-)

--
2.15.1