[PATCH 0/3] SGI-XPC: Adjustments for some function implementations

[PATCH 0/3] SGI-XPC: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 10 Jan 2018 14:54:34 +0100

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

Markus Elfring (3):
  Delete error messages for a failed memory allocation in three functions
  Improve a size determination in two functions
  Adjust 27 checks for null pointers

 drivers/misc/sgi-xp/xpc_main.c | 16 +++++-------
 drivers/misc/sgi-xp/xpc_uv.c   | 59 ++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 44 deletions(-)

-- 
2.15.1

[PATCH 1/3] sgi-xpc: Delete error messages for a failed memory allocation in three functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 10 Jan 2018 14:00:17 +0100

Omit extra messages for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/misc/sgi-xp/xpc_main.c | 8 ++------
 drivers/misc/sgi-xp/xpc_uv.c   | 4 ----
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/sgi-xp/xpc_main.c b/drivers/misc/sgi-xp/xpc_main.c
index 0c775d6fcf59..c90a9ff30680 100644
--- a/drivers/misc/sgi-xp/xpc_main.c
+++ b/drivers/misc/sgi-xp/xpc_main.c
@@ -418,10 +418,8 @@ xpc_setup_ch_structures(struct xpc_partition *part)
 	DBUG_ON(part->channels != NULL);
 	part->channels = kzalloc(sizeof(struct xpc_channel) * XPC_MAX_NCHANNELS,
 				 GFP_KERNEL);
-	if (part->channels = NULL) {
-		dev_err(xpc_chan, "can't get memory for channels\n");
+	if (!part->channels)
 		return xpNoMemory;
-	}
 
 	/* allocate the remote open and close args */
 
@@ -907,10 +905,8 @@ xpc_setup_partitions(void)
 
 	xpc_partitions = kzalloc(sizeof(struct xpc_partition) *
 				 xp_max_npartitions, GFP_KERNEL);
-	if (xpc_partitions = NULL) {
-		dev_err(xpc_part, "can't get memory for partition structure\n");
+	if (!xpc_partitions)
 		return -ENOMEM;
-	}
 
 	/*
 	 * The first few fields of each entry of xpc_partitions[] need to
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 340b44d9e8cf..0c95f9994649 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -217,8 +217,6 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 
 	mq = kmalloc(sizeof(struct xpc_gru_mq_uv), GFP_KERNEL);
 	if (mq = NULL) {
-		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to kmalloc() "
-			"a xpc_gru_mq_uv structure\n");
 		ret = -ENOMEM;
 		goto out_0;
 	}
@@ -226,8 +224,6 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 	mq->gru_mq_desc = kzalloc(sizeof(struct gru_message_queue_desc),
 				  GFP_KERNEL);
 	if (mq->gru_mq_desc = NULL) {
-		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to kmalloc() "
-			"a gru_message_queue_desc structure\n");
 		ret = -ENOMEM;
 		goto out_1;
 	}
-- 
2.15.1

[PATCH 2/3] sgi-xpc: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 10 Jan 2018 14:18:12 +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 <elfring@users.sourceforge.net>
---
 drivers/misc/sgi-xp/xpc_uv.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 0c95f9994649..8a8dfcbbe729 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -212,10 +212,9 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 	int nasid;
 	int pg_order;
 	struct page *page;
-	struct xpc_gru_mq_uv *mq;
 	struct uv_IO_APIC_route_entry *mmr_value;
+	struct xpc_gru_mq_uv *mq = kmalloc(sizeof(*mq), GFP_KERNEL);
 
-	mq = kmalloc(sizeof(struct xpc_gru_mq_uv), GFP_KERNEL);
 	if (mq = NULL) {
 		ret = -ENOMEM;
 		goto out_0;
@@ -686,9 +685,7 @@ xpc_send_activate_IRQ_uv(struct xpc_partition *part, void *msg, size_t msg_size,
 	if (!(part_uv->flags & XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV)) {
 		gru_mq_desc = part_uv->cached_activate_gru_mq_desc;
 		if (gru_mq_desc = NULL) {
-			gru_mq_desc = kmalloc(sizeof(struct
-					      gru_message_queue_desc),
-					      GFP_KERNEL);
+			gru_mq_desc = kmalloc(sizeof(*gru_mq_desc), GFP_KERNEL);
 			if (gru_mq_desc = NULL) {
 				ret = xpNoMemory;
 				goto done;
-- 
2.15.1

[PATCH 3/3] sgi-xpc: Adjust 27 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 10 Jan 2018 14:40:57 +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 <elfring@users.sourceforge.net>
---
 drivers/misc/sgi-xp/xpc_main.c |  8 +++----
 drivers/misc/sgi-xp/xpc_uv.c   | 48 ++++++++++++++++++++----------------------
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/misc/sgi-xp/xpc_main.c b/drivers/misc/sgi-xp/xpc_main.c
index c90a9ff30680..ad8b970e1429 100644
--- a/drivers/misc/sgi-xp/xpc_main.c
+++ b/drivers/misc/sgi-xp/xpc_main.c
@@ -383,7 +383,7 @@ xpc_kzalloc_cacheline_aligned(size_t size, gfp_t flags, void **base)
 {
 	/* see if kzalloc will give us cachline aligned memory by default */
 	*base = kzalloc(size, flags);
-	if (*base = NULL)
+	if (!*base)
 		return NULL;
 
 	if ((u64)*base = L1_CACHE_ALIGN((u64)*base))
@@ -393,7 +393,7 @@ xpc_kzalloc_cacheline_aligned(size_t size, gfp_t flags, void **base)
 
 	/* nope, we'll have to do it ourselves */
 	*base = kzalloc(size + L1_CACHE_BYTES, flags);
-	if (*base = NULL)
+	if (!*base)
 		return NULL;
 
 	return (void *)L1_CACHE_ALIGN((u64)*base);
@@ -415,7 +415,7 @@ xpc_setup_ch_structures(struct xpc_partition *part)
 	 * Allocate all of the channel structures as a contiguous chunk of
 	 * memory.
 	 */
-	DBUG_ON(part->channels != NULL);
+	DBUG_ON(part->channels);
 	part->channels = kzalloc(sizeof(struct xpc_channel) * XPC_MAX_NCHANNELS,
 				 GFP_KERNEL);
 	if (!part->channels)
@@ -427,7 +427,7 @@ xpc_setup_ch_structures(struct xpc_partition *part)
 	    xpc_kzalloc_cacheline_aligned(XPC_OPENCLOSE_ARGS_SIZE,
 					  GFP_KERNEL, &part->
 					  remote_openclose_args_base);
-	if (part->remote_openclose_args = NULL) {
+	if (!part->remote_openclose_args) {
 		dev_err(xpc_chan, "can't get memory for remote connect args\n");
 		ret = xpNoMemory;
 		goto out_1;
diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
index 8a8dfcbbe729..d085bc0b025e 100644
--- a/drivers/misc/sgi-xp/xpc_uv.c
+++ b/drivers/misc/sgi-xp/xpc_uv.c
@@ -91,8 +91,7 @@ xpc_teardown_partitions_uv(void)
 
 	for (partid = 0; partid < XP_MAX_NPARTITIONS_UV; partid++) {
 		part_uv = &xpc_partitions[partid].sn.uv;
-
-		if (part_uv->cached_activate_gru_mq_desc != NULL) {
+		if (part_uv->cached_activate_gru_mq_desc) {
 			mutex_lock(&part_uv->cached_activate_gru_mq_desc_mutex);
 			spin_lock_irqsave(&part_uv->flags_lock, irq_flags);
 			part_uv->flags &= ~XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV;
@@ -215,14 +214,14 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 	struct uv_IO_APIC_route_entry *mmr_value;
 	struct xpc_gru_mq_uv *mq = kmalloc(sizeof(*mq), GFP_KERNEL);
 
-	if (mq = NULL) {
+	if (!mq) {
 		ret = -ENOMEM;
 		goto out_0;
 	}
 
 	mq->gru_mq_desc = kzalloc(sizeof(struct gru_message_queue_desc),
 				  GFP_KERNEL);
-	if (mq->gru_mq_desc = NULL) {
+	if (!mq->gru_mq_desc) {
 		ret = -ENOMEM;
 		goto out_1;
 	}
@@ -237,7 +236,7 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
 	page = __alloc_pages_node(nid,
 				      GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
 				      pg_order);
-	if (page = NULL) {
+	if (!page) {
 		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to alloc %d "
 			"bytes of memory on nid=%d for GRU mq\n", mq_size, nid);
 		ret = -ENOMEM;
@@ -621,7 +620,7 @@ xpc_handle_activate_IRQ_uv(int irq, void *dev_id)
 
 	while (1) {
 		msg_hdr = gru_get_next_message(xpc_activate_mq_uv->gru_mq_desc);
-		if (msg_hdr = NULL)
+		if (!msg_hdr)
 			break;
 
 		partid = msg_hdr->partid;
@@ -684,9 +683,9 @@ xpc_send_activate_IRQ_uv(struct xpc_partition *part, void *msg, size_t msg_size,
 again:
 	if (!(part_uv->flags & XPC_P_CACHED_ACTIVATE_GRU_MQ_DESC_UV)) {
 		gru_mq_desc = part_uv->cached_activate_gru_mq_desc;
-		if (gru_mq_desc = NULL) {
+		if (!gru_mq_desc) {
 			gru_mq_desc = kmalloc(sizeof(*gru_mq_desc), GFP_KERNEL);
-			if (gru_mq_desc = NULL) {
+			if (!gru_mq_desc) {
 				ret = xpNoMemory;
 				goto done;
 			}
@@ -737,12 +736,12 @@ xpc_send_activate_IRQ_ch_uv(struct xpc_channel *ch, unsigned long *irq_flags,
 
 	ret = xpc_send_activate_IRQ_uv(part, msg, msg_size, msg_type);
 	if (unlikely(ret != xpSuccess)) {
-		if (irq_flags != NULL)
+		if (irq_flags)
 			spin_unlock_irqrestore(&ch->lock, *irq_flags);
 
 		XPC_DEACTIVATE_PARTITION(part, ret);
 
-		if (irq_flags != NULL)
+		if (irq_flags)
 			spin_lock_irqsave(&ch->lock, *irq_flags);
 	}
 }
@@ -961,9 +960,9 @@ xpc_get_fifo_entry_uv(struct xpc_fifo_head_uv *head)
 
 	spin_lock_irqsave(&head->lock, irq_flags);
 	first = head->first;
-	if (head->first != NULL) {
+	if (head->first) {
 		head->first = first->next;
-		if (head->first = NULL)
+		if (!head->first)
 			head->last = NULL;
 
 		head->n_entries--;
@@ -983,7 +982,7 @@ xpc_put_fifo_entry_uv(struct xpc_fifo_head_uv *head,
 
 	last->next = NULL;
 	spin_lock_irqsave(&head->lock, irq_flags);
-	if (head->last != NULL)
+	if (head->last)
 		head->last->next = last;
 	else
 		head->first = last;
@@ -1084,7 +1083,7 @@ xpc_allocate_send_msg_slot_uv(struct xpc_channel *ch)
 	for (nentries = ch->local_nentries; nentries > 0; nentries--) {
 		nbytes = nentries * sizeof(struct xpc_send_msg_slot_uv);
 		ch_uv->send_msg_slots = kzalloc(nbytes, GFP_KERNEL);
-		if (ch_uv->send_msg_slots = NULL)
+		if (!ch_uv->send_msg_slots)
 			continue;
 
 		for (entry = 0; entry < nentries; entry++) {
@@ -1118,7 +1117,7 @@ xpc_allocate_recv_msg_slot_uv(struct xpc_channel *ch)
 	for (nentries = ch->remote_nentries; nentries > 0; nentries--) {
 		nbytes = nentries * ch->entry_size;
 		ch_uv->recv_msg_slots = kzalloc(nbytes, GFP_KERNEL);
-		if (ch_uv->recv_msg_slots = NULL)
+		if (!ch_uv->recv_msg_slots)
 			continue;
 
 		for (entry = 0; entry < nentries; entry++) {
@@ -1152,7 +1151,7 @@ xpc_setup_msg_structures_uv(struct xpc_channel *ch)
 	ch_uv->cached_notify_gru_mq_desc = kmalloc(sizeof(struct
 						   gru_message_queue_desc),
 						   GFP_KERNEL);
-	if (ch_uv->cached_notify_gru_mq_desc = NULL)
+	if (!ch_uv->cached_notify_gru_mq_desc)
 		return xpNoMemory;
 
 	ret = xpc_allocate_send_msg_slot_uv(ch);
@@ -1263,7 +1262,7 @@ xpc_save_remote_msgqueue_pa_uv(struct xpc_channel *ch,
 {
 	struct xpc_channel_uv *ch_uv = &ch->sn.uv;
 
-	DBUG_ON(ch_uv->cached_notify_gru_mq_desc = NULL);
+	DBUG_ON(!ch_uv->cached_notify_gru_mq_desc);
 	return xpc_cache_remote_gru_mq_desc_uv(ch_uv->cached_notify_gru_mq_desc,
 					       gru_mq_desc_gpa);
 }
@@ -1327,7 +1326,7 @@ xpc_allocate_msg_slot_uv(struct xpc_channel *ch, u32 flags,
 
 	while (1) {
 		entry = xpc_get_fifo_entry_uv(&ch->sn.uv.msg_slot_free_list);
-		if (entry != NULL)
+		if (entry)
 			break;
 
 		if (flags & XPC_NOWAIT)
@@ -1361,8 +1360,7 @@ xpc_notify_sender_uv(struct xpc_channel *ch,
 {
 	xpc_notify_func func = msg_slot->func;
 
-	if (func != NULL && cmpxchg(&msg_slot->func, func, NULL) = func) {
-
+	if (func && cmpxchg(&msg_slot->func, func, NULL) = func) {
 		atomic_dec(&ch->n_to_notify);
 
 		dev_dbg(xpc_chan, "msg_slot->func() called, msg_slot=0x%p "
@@ -1389,7 +1387,7 @@ xpc_handle_notify_mq_ack_uv(struct xpc_channel *ch,
 	BUG_ON(msg_slot->msg_slot_number != msg->hdr.msg_slot_number);
 	msg_slot->msg_slot_number += ch->local_nentries;
 
-	if (msg_slot->func != NULL)
+	if (msg_slot->func)
 		xpc_notify_sender_uv(ch, msg_slot, xpMsgDelivered);
 
 	xpc_free_msg_slot_uv(ch, msg_slot);
@@ -1551,7 +1549,7 @@ xpc_send_payload_uv(struct xpc_channel *ch, u32 flags, void *payload,
 	if (ret != xpSuccess)
 		goto out_1;
 
-	if (func != NULL) {
+	if (func) {
 		atomic_inc(&ch->n_to_notify);
 
 		msg_slot->key = key;
@@ -1578,7 +1576,7 @@ xpc_send_payload_uv(struct xpc_channel *ch, u32 flags, void *payload,
 
 	XPC_DEACTIVATE_PARTITION(&xpc_partitions[ch->partid], ret);
 out_2:
-	if (func != NULL) {
+	if (func) {
 		/*
 		 * Try to NULL the msg_slot's func field. If we fail, then
 		 * xpc_notify_senders_of_disconnect_uv() beat us to it, in which
@@ -1624,7 +1622,7 @@ xpc_notify_senders_of_disconnect_uv(struct xpc_channel *ch)
 			break;
 
 		msg_slot = &ch->sn.uv.send_msg_slots[entry];
-		if (msg_slot->func != NULL)
+		if (msg_slot->func)
 			xpc_notify_sender_uv(ch, msg_slot, ch->reason);
 	}
 }
@@ -1641,7 +1639,7 @@ xpc_get_deliverable_payload_uv(struct xpc_channel *ch)
 
 	if (!(ch->flags & XPC_C_DISCONNECTING)) {
 		entry = xpc_get_fifo_entry_uv(&ch->sn.uv.recv_msg_list);
-		if (entry != NULL) {
+		if (entry) {
 			msg = container_of(entry, struct xpc_notify_mq_msg_uv,
 					   hdr.u.next);
 			payload = &msg->payload;
-- 
2.15.1

Re: [PATCH 1/3] sgi-xpc: Delete error messages for a failed memory allocation in three functions

[Robin Holt]

On Wed, Jan 10, 2018 at 8:04 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 10 Jan 2018 14:00:17 +0100
>
> Omit extra messages for a memory allocation failure in these functions.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Robin Holt <robinmholt@gmail.com>

Re: [PATCH 2/3] sgi-xpc: Improve a size determination in two functions

[Robin Holt]

On Wed, Jan 10, 2018 at 8:05 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Robin Holt <robinmholt@gmail.com>

Re: [PATCH 3/3] sgi-xpc: Adjust 27 checks for null pointers

[Robin Holt]

On Wed, Jan 10, 2018 at 8:06 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Acked-by: Robin Holt <robinmholt@gmail.com>

Re: [PATCH 0/3] SGI-XPC: Adjustments for some function implementations

[Robin Holt]

If you need to resubmit, please consider re-ordering the changes to
due the comparison to NULL changes in your first patch, followed by the
other two in either order.  I think it will clear up some of the inconsistency
in your first patch.

Thanks,
Robin

Re: [PATCH 0/3] SGI-XPC: Adjustments for some function implementations

[Greg Kroah-Hartman]

On Thu, Jan 11, 2018 at 12:19:37PM -0600, Robin Holt wrote:
> If you need to resubmit, please consider re-ordering the changes to
> due the comparison to NULL changes in your first patch, followed by the
> other two in either order.  I think it will clear up some of the inconsistency
> in your first patch.

Note you are responding to someone who is on my, and many other kernel
maintainers, blacklists, so I never see the patches.  So it's not really
worth reviewing, as I'll not be picking them up myself, sorry.

greg k-h