Skip to contentSkip to conversation

message: fusion: Fix clang and checkpatch warnings

PATCH Superseded Varsha Rao 2 participants 5 messages Jun 3, 2018

Subsystems (2)
SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Varsha Rao

author
#0 lore raw

cover message: fusion: Fix clang and checkpatch warnings

This patchset fixes extraneous parentheses clang warning and few checkpatch issues.

Varsha Rao (3):
message: fusion: Remove extra parentheses
message: fusion: Replace NULL comparison
message: fusion: Fix line over 80 characters

 drivers/message/fusion/mptbase.c | 50 ++++++++++++++++----------------
 1 file changed, 25 insertions(+), 25 deletions(-)
Signature · 1 line
2.17.0
author
#1 lore raw

PATCH 1/3 message: fusion: Remove extra parentheses

To fix clang warning of extraneous parentheses and check patch issue, remove extra parentheses and replace x == NULL with !x. Following coccinele script is used to fix it.

@disable is_null,paren@
expression e;
statement s;
@@
if (
- (e==NULL)
+!e
)
s

Signed-off-by Varsha Rao <[email protected]>
---
 drivers/message/fusion/mptbase.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/message/fusion/mptbase.c +2 -2
index 51eb1b027963..3fe53b65c5fa 100644
@@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg)
 	MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg;
 	struct pci_dev *pdev;
 
-	if ((ioc == NULL))
+	if (!ioc)
 		return -1;
 
 	pdev = ioc->pcidev;
-	if ((pdev == NULL))
+	if (!pdev)
 		return -1;
 
 	pci_stop_and_remove_bus_device_locked(pdev);
Signature · 1 line
2.17.0
author
#2 lore raw

PATCH 2/3 message: fusion: Replace NULL comparison

In reply to Varsha Rao

Replace x == NULL with !x, to fix checkpatch issue. Following coccinelle script is used to fix it.

@disable is_null@
expression e;
@@
- e == NULL
+!e

Signed-off-by Varsha Rao <[email protected]>
---
 drivers/message/fusion/mptbase.c | 45 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 23 deletions(-)
drivers/message/fusion/mptbase.c +22 -23
index 3fe53b65c5fa..2e5e5487f193 100644
@@ -488,7 +488,7 @@ mpt_turbo_reply(MPT_ADAPTER *ioc, u32 pa)
 
 	/*  Check for (valid) IO callback!  */
 	if (!cb_idx || cb_idx >= MPT_MAX_PROTOCOL_DRIVERS ||
-		MptCallbacks[cb_idx] == NULL) {
+		!MptCallbacks[cb_idx]) {
 		printk(MYIOC_s_WARN_FMT "%s: Invalid cb_idx (%d)!\n",
 				__func__, ioc->name, cb_idx);
 		goto out;

@@ -552,7 +552,7 @@ mpt_reply(MPT_ADAPTER *ioc, u32 pa)
 
 	/*  Check for (valid) IO callback!  */
 	if (!cb_idx || cb_idx >= MPT_MAX_PROTOCOL_DRIVERS ||
-		MptCallbacks[cb_idx] == NULL) {
+		!MptCallbacks[cb_idx]) {
 		printk(MYIOC_s_WARN_FMT "%s: Invalid cb_idx (%d)!\n",
 				__func__, ioc->name, cb_idx);
 		freeme = 0;

@@ -707,7 +707,7 @@ mpt_register(MPT_CALLBACK cbfunc, MPT_DRIVER_CLASS dclass, char *func_name)
 	 *  (slot/handle 0 is reserved!)
 	 */
 	for (cb_idx = MPT_MAX_PROTOCOL_DRIVERS-1; cb_idx; cb_idx--) {
-		if (MptCallbacks[cb_idx] == NULL) {
+		if (!MptCallbacks[cb_idx]) {
 			MptCallbacks[cb_idx] = cbfunc;
 			MptDriverClass[cb_idx] = dclass;
 			MptEvHandlers[cb_idx] = NULL;

@@ -928,7 +928,7 @@ mpt_get_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc)
 	spin_unlock_irqrestore(&ioc->FreeQlock, flags);
 
 #ifdef MFCNT
-	if (mf == NULL)
+	if (!mf)
 		printk(MYIOC_s_WARN_FMT "IOC Active. No free Msg Frames! "
 		    "Count 0x%x Max 0x%x\n", ioc->name, ioc->mfcnt,
 		    ioc->req_depth);

@@ -1723,7 +1723,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	/* Get logical ptr for PciMem0 space */
 	/*mem = ioremap(mem_phys, msize);*/
 	mem = ioremap(mem_phys, msize);
-	if (mem == NULL) {
+	if (!mem) {
 		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
 			" memory!\n", ioc->name);
 		r = -EINVAL;

@@ -1780,7 +1780,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 #endif
 
 	ioc = kzalloc(sizeof(MPT_ADAPTER), GFP_ATOMIC);
-	if (ioc == NULL) {
+	if (!ioc) {
 		printk(KERN_ERR MYNAM ": ERROR - Insufficient memory to add adapter!\n");
 		return -ENOMEM;
 	}

@@ -2819,7 +2819,7 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 {
 	int sz_first, sz_last;
 
-	if (ioc == NULL)
+	if (!ioc)
 		return;
 
 	sz_first = ioc->alloc_total;

@@ -4326,17 +4326,17 @@ initChainBuffers(MPT_ADAPTER *ioc)
 	/* ReqToChain size must equal the req_depth
 	 * index = req_idx
 	 */
-	if (ioc->ReqToChain == NULL) {
+	if (!ioc->ReqToChain) {
 		sz = ioc->req_depth * sizeof(int);
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
+		if (!mem)
 			return -1;
 
 		ioc->ReqToChain = (int *) mem;
 		dinitprintk(ioc, printk(MYIOC_s_DEBUG_FMT "ReqToChain alloc  @ %p, sz=%d bytes\n",
 			 	ioc->name, mem, sz));
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
+		if (!mem)
 			return -1;
 
 		ioc->RequestNB = (int *) mem;

@@ -4401,9 +4401,9 @@ initChainBuffers(MPT_ADAPTER *ioc)
 	ioc->num_chain = num_chain;
 
 	sz = num_chain * sizeof(int);
-	if (ioc->ChainToChain == NULL) {
+	if (!ioc->ChainToChain) {
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
+		if (!mem)
 			return -1;
 
 		ioc->ChainToChain = (int *) mem;

@@ -4441,7 +4441,7 @@ PrimeIocFifos(MPT_ADAPTER *ioc)
 
 	/*  Prime reply FIFO...  */
 
-	if (ioc->reply_frames == NULL) {
+	if (!ioc->reply_frames) {
 		if ( (num_chain = initChainBuffers(ioc)) < 0)
 			return -1;
 		/*

@@ -4493,7 +4493,7 @@ PrimeIocFifos(MPT_ADAPTER *ioc)
 
 		total_size += sz;
 		mem = pci_alloc_consistent(ioc->pcidev, total_size, &alloc_dma);
-		if (mem == NULL) {
+		if (!mem) {
 			printk(MYIOC_s_ERR_FMT "Unable to allocate Reply, Request, Chain Buffers!\n",
 				ioc->name);
 			goto out_fail;

@@ -4571,7 +4571,7 @@ PrimeIocFifos(MPT_ADAPTER *ioc)
 		sz = (ioc->req_depth * MPT_SENSE_BUFFER_ALLOC);
 		ioc->sense_buf_pool =
 			pci_alloc_consistent(ioc->pcidev, sz, &ioc->sense_buf_pool_dma);
-		if (ioc->sense_buf_pool == NULL) {
+		if (!ioc->sense_buf_pool) {
 			printk(MYIOC_s_ERR_FMT "Unable to allocate Sense Buffers!\n",
 				ioc->name);
 			goto out_fail;

@@ -5090,7 +5090,7 @@ mptbase_sas_persist_operation(MPT_ADAPTER *ioc, u8 persist_opcode)
 
 	/* Get a MF for this command.
 	 */
-	if ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	if (!(mf = mpt_get_msg_frame(mpt_base_index, ioc))) {
 		printk(KERN_DEBUG "%s: no msg frames!\n", __func__);
 		ret = -1;
 		goto out;

@@ -5370,7 +5370,7 @@ mpt_GetScsiPortSettings(MPT_ADAPTER *ioc, int portnum)
 		u8	*mem;
 		sz = MPT_MAX_SCSI_DEVICES * sizeof(int);
 		mem = kmalloc(sz, GFP_ATOMIC);
-		if (mem == NULL)
+		if (!mem)
 			return -EFAULT;
 
 		ioc->spi_data.nvram = (int *) mem;

@@ -5690,8 +5690,7 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 		    buffer->PhysDisk[i].PhysDiskNum, &phys_disk) != 0)
 			continue;
 
-		if ((component_info = kmalloc(sizeof (*component_info),
-		 GFP_KERNEL)) == NULL)
+		if (!(component_info = kmalloc(sizeof(*component_info), GFP_KERNEL)))
 			continue;
 
 		component_info->volumeID = id;

@@ -6104,7 +6103,7 @@ mpt_read_ioc_pg_4(MPT_ADAPTER *ioc)
 	if (header.PageLength == 0)
 		return;
 
-	if ( (pIoc4 = ioc->spi_data.pIocPg4) == NULL ) {
+	if (!(pIoc4 = ioc->spi_data.pIocPg4)) {
 		iocpage4sz = (header.PageLength + 4) * 4; /* Allow 4 additional SEP's */
 		pIoc4 = pci_alloc_consistent(ioc->pcidev, iocpage4sz, &ioc4_dma);
 		if (!pIoc4)

@@ -6300,7 +6299,7 @@ SendEventAck(MPT_ADAPTER *ioc, EventNotificationReply_t *evnp)
 {
 	EventAck_t	*pAck;
 
-	if ((pAck = (EventAck_t *) mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	if (!(pAck = (EventAck_t *)mpt_get_msg_frame(mpt_base_index, ioc))) {
 		dfailprintk(ioc, printk(MYIOC_s_WARN_FMT "%s, no msg frames!!\n",
 		    ioc->name, __func__));
 		return -1;

@@ -6391,7 +6390,7 @@ mpt_config(MPT_ADAPTER *ioc, CONFIGPARMS *pCfg)
 
 	/* Get and Populate a free Frame
 	 */
-	if ((mf = mpt_get_msg_frame(mpt_base_index, ioc)) == NULL) {
+	if (!(mf = mpt_get_msg_frame(mpt_base_index, ioc))) {
 		dcprintk(ioc, printk(MYIOC_s_WARN_FMT
 		"mpt_config: no msg frames!\n", ioc->name));
 		ret = -EAGAIN;

@@ -6603,7 +6602,7 @@ static int
 procmpt_create(void)
 {
 	mpt_proc_root_dir = proc_mkdir(MPT_PROCFS_MPTBASEDIR, NULL);
-	if (mpt_proc_root_dir == NULL)
+	if (!mpt_proc_root_dir)
 		return -ENOTDIR;
 
 	proc_create("summary", S_IRUGO, mpt_proc_root_dir, &mpt_summary_proc_fops);
Signature · 1 line
2.17.0
author
#3 lore raw

PATCH 3/3 message: fusion: Fix line over 80 characters

In reply to Varsha Rao

Line over 80 characters is split to fix the following checkpatch issue:

WARNING: line over 80 characters

Signed-off-by Varsha Rao <[email protected]>
---
 drivers/message/fusion/mptbase.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/message/fusion/mptbase.c +2 -1
index 2e5e5487f193..49e5d1ae9ea9 100644
@@ -5690,7 +5690,8 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 		    buffer->PhysDisk[i].PhysDiskNum, &phys_disk) != 0)
 			continue;
 
-		if (!(component_info = kmalloc(sizeof(*component_info), GFP_KERNEL)))
+		if (!(component_info = kmalloc(sizeof(*component_info),
+		      GFP_KERNEL)))
 			continue;
 
 		component_info->volumeID = id;
Signature · 1 line
2.17.0
#4 lore raw

Re: [PATCH 3/3] message: fusion: Fix line over 80 characters

Quoted 3 lines · Varsha Rao

Line over 80 characters is split to fix the following checkpatch issue:

WARNING: line over 80 characters

[]

drivers/message/fusion/mptbase.c

Quoted 1 line
drivers/message/fusion/mptbase.c +0 -0

[]

drivers/message/fusion/mptbase.c mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id) · line 5695

Quoted 8 lines
drivers/message/fusion/mptbase.c +2 -1
@@ -5690,7 +5690,8 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 		    buffer->PhysDisk[i].PhysDiskNum, &phys_disk) != 0)
 			continue;
 
-		if (!(component_info = kmalloc(sizeof(*component_info), GFP_KERNEL)))
+		if (!(component_info = kmalloc(sizeof(*component_info),
+		      GFP_KERNEL)))
 			continue;

It's much more common to split this into multiple statements like

		component_info = kmalloc(sizeof(*component_info), GFP_KERNEL);
		if (!component_info)
			continue;

but the continue in a loop with a known alloc failure seems silly as the next alloc is also extremely likely to fail as well.

btw: Are you actually using this code?

If not, perhaps it's better to send cleanup only patches for files in drivers/staging as practice instead.