[PATCH 0/3] GenWQE: Adjustments for some function implementations

[PATCH 0/3] GenWQE: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 Jan 2018 10:32:23 +0100

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

Markus Elfring (3):
  Delete an error message for a failed memory allocation in genwqe_user_vmap()
  Fix a typo in two comments
  Adjust 12 checks for null pointers

 drivers/misc/genwqe/card_utils.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

-- 
2.15.1

[PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 Jan 2018 09:37:23 +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 <elfring@users.sourceforge.net>
---
 drivers/misc/genwqe/card_utils.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 8f2e6442d88b..55c389a9e7d7 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
 			       sizeof(struct page *) + sizeof(dma_addr_t),
 			       GFP_KERNEL);
 	if (!m->page_list) {
-		dev_err(&pci_dev->dev, "err: alloc page_list failed\n");
 		m->nr_pages = 0;
 		m->u_vaddr = NULL;
 		m->size = 0;	/* mark unused and not added */
-- 
2.15.1

Re: [PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[haver]

Hi Markus,

On 2018-01-08 10:41, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 Jan 2018 09:37:23 +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 <elfring@users.sourceforge.net>
> ---
>  drivers/misc/genwqe/card_utils.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/misc/genwqe/card_utils.c 
> b/drivers/misc/genwqe/card_utils.c
> index 8f2e6442d88b..55c389a9e7d7 100644
> --- a/drivers/misc/genwqe/card_utils.c
> +++ b/drivers/misc/genwqe/card_utils.c
> @@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct
> dma_mapping *m, void *uaddr,
>  			       sizeof(struct page *) + sizeof(dma_addr_t),
>  			       GFP_KERNEL);
>  	if (!m->page_list) {
> -		dev_err(&pci_dev->dev, "err: alloc page_list failed\n");

Are there different printouts which cover this? I mean the debug 
printout is not
appearing all the time, it must be enabled, if I remember correctly. So 
why do
you suggest to remove it?

>  		m->nr_pages = 0;
>  		m->u_vaddr = NULL;
>  		m->size = 0;	/* mark unused and not added */

Regards

Frank

Re: [PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[Dan Carpenter]

On Mon, Jan 08, 2018 at 01:45:02PM +0100, haver wrote:
> Hi Markus,
> 
> On 2018-01-08 10:41, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Mon, 8 Jan 2018 09:37:23 +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 <elfring@users.sourceforge.net>
> > ---
> >  drivers/misc/genwqe/card_utils.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/misc/genwqe/card_utils.c
> > b/drivers/misc/genwqe/card_utils.c
> > index 8f2e6442d88b..55c389a9e7d7 100644
> > --- a/drivers/misc/genwqe/card_utils.c
> > +++ b/drivers/misc/genwqe/card_utils.c
> > @@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct
> > dma_mapping *m, void *uaddr,
> >  			       sizeof(struct page *) + sizeof(dma_addr_t),
> >  			       GFP_KERNEL);
> >  	if (!m->page_list) {
> > -		dev_err(&pci_dev->dev, "err: alloc page_list failed\n");
> 
> Are there different printouts which cover this? I mean the debug printout is
> not
> appearing all the time, it must be enabled, if I remember correctly.

The kmalloc() error messages are enabled by default.  See warn_alloc().
The stack trace from this specific call site is going to be very clear
what went wrong.


> So why
> do
> you suggest to remove it?
>

It's a checkpatch warning and a small memory savings.

regards,
dan carpenter

Re: [PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[haver]

Hi Dan,

On 2018-01-08 14:23, Dan Carpenter wrote:
> On Mon, Jan 08, 2018 at 01:45:02PM +0100, haver wrote:
>> Hi Markus,
>> 
>> On 2018-01-08 10:41, SF Markus Elfring wrote:
>> > From: Markus Elfring <elfring@users.sourceforge.net>
>> > Date: Mon, 8 Jan 2018 09:37:23 +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 <elfring@users.sourceforge.net>
>> > ---
>> >  drivers/misc/genwqe/card_utils.c | 1 -
>> >  1 file changed, 1 deletion(-)
>> >
>> > diff --git a/drivers/misc/genwqe/card_utils.c
>> > b/drivers/misc/genwqe/card_utils.c
>> > index 8f2e6442d88b..55c389a9e7d7 100644
>> > --- a/drivers/misc/genwqe/card_utils.c
>> > +++ b/drivers/misc/genwqe/card_utils.c
>> > @@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct
>> > dma_mapping *m, void *uaddr,
>> >  			       sizeof(struct page *) + sizeof(dma_addr_t),
>> >  			       GFP_KERNEL);
>> >  	if (!m->page_list) {
>> > -		dev_err(&pci_dev->dev, "err: alloc page_list failed\n");
>> 
>> Are there different printouts which cover this? I mean the debug 
>> printout is
>> not
>> appearing all the time, it must be enabled, if I remember correctly.
> 
> The kmalloc() error messages are enabled by default.  See warn_alloc().
> The stack trace from this specific call site is going to be very clear
> what went wrong.
> 

So it appears it is ok to be removed. Let's remove the redundant 
printout as suggested.

> 
>> So why
>> do
>> you suggest to remove it?
>> 
> 
> It's a checkpatch warning and a small memory savings.
> 
> regards,
> dan carpenter

Acked-by: Frank Haverkamp <haver@linux.vnet.ibm.com>

Re: [PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[SF Markus Elfring]

>> @@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct
>> dma_mapping *m, void *uaddr,
>>                     sizeof(struct page *) + sizeof(dma_addr_t),
>>                     GFP_KERNEL);
>>      if (!m->page_list) {
>> -        dev_err(&pci_dev->dev, "err: alloc page_list failed\n");
> 
> Are there different printouts which cover this?

Is this error message redundant?


> I mean the debug printout is not appearing all the time,
> it must be enabled, if I remember correctly.

Would you like to clarify corresponding configuration possibilities any more?


> So why do you suggest to remove it?

Can the Linux allocation failure report be sufficient for this use case already?

Regards,
Markus

Re: [PATCH 1/3] GenWQE: Delete an error message for a failed memory allocation in genwqe_user_vmap()

[haver]

On 2018-01-08 14:24, SF Markus Elfring wrote:
>>> @@ -593,7 +593,6 @@ int genwqe_user_vmap(struct genwqe_dev *cd, 
>>> struct
>>> dma_mapping *m, void *uaddr,
>>>                     sizeof(struct page *) + 
>>> sizeof(dma_addr_t),
>>>                     GFP_KERNEL);
>>>      if (!m->page_list) {
>>> -        dev_err(&pci_dev->dev, "err: alloc page_list 
>>> failed\n");
>> 
>> Are there different printouts which cover this?
> 
> Is this error message redundant?
> 
> 
>> I mean the debug printout is not appearing all the time,
>> it must be enabled, if I remember correctly.
> 
> Would you like to clarify corresponding configuration possibilities any 
> more?
> 
> 
>> So why do you suggest to remove it?
> 
> Can the Linux allocation failure report be sufficient for this use case 
> already?
> 
> Regards,
> Markus

According to Dans message in parallel, it should be redundant and I am 
fine with removing it.

Acked-by: Frank Haverkamp <haver@linux.vnet.ibm.com>

[PATCH 2/3] GenWQE: Fix a typo in two comments

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 Jan 2018 09:57:10 +0100

Add a missing character in two words of these descriptions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/misc/genwqe/card_utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 55c389a9e7d7..0b466664c908 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -453,7 +453,7 @@ int genwqe_setup_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
 		s += 8;		/* continue 8 elements further */
 	}
  fixup:
-	if (j == 1) {		/* combining happend on last entry! */
+	if (j == 1) {		/* combining happened on last entry! */
 		s -= 8;		/* full shift needed on previous sgl block */
 		j =  7;		/* shift all elements */
 	}
@@ -471,7 +471,7 @@ int genwqe_setup_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
  * genwqe_free_sync_sgl() - Free memory for sgl and overlapping pages
  *
  * After the DMA transfer has been completed we free the memory for
- * the sgl and the cached pages. Data is being transfered from cached
+ * the sgl and the cached pages. Data is being transferred from cached
  * pages into user-space buffers.
  */
 int genwqe_free_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl)
-- 
2.15.1

Re: [PATCH 2/3] GenWQE: Fix a typo in two comments

[haver]

Hi Markus,

On 2018-01-08 10:42, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 Jan 2018 09:57:10 +0100
> 
> Add a missing character in two words of these descriptions.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/misc/genwqe/card_utils.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/genwqe/card_utils.c 
> b/drivers/misc/genwqe/card_utils.c
> index 55c389a9e7d7..0b466664c908 100644
> --- a/drivers/misc/genwqe/card_utils.c
> +++ b/drivers/misc/genwqe/card_utils.c
> @@ -453,7 +453,7 @@ int genwqe_setup_sgl(struct genwqe_dev *cd, struct
> genwqe_sgl *sgl,
>  		s += 8;		/* continue 8 elements further */
>  	}
>   fixup:
> -	if (j == 1) {		/* combining happend on last entry! */
> +	if (j == 1) {		/* combining happened on last entry! */
>  		s -= 8;		/* full shift needed on previous sgl block */
>  		j =  7;		/* shift all elements */
>  	}
> @@ -471,7 +471,7 @@ int genwqe_setup_sgl(struct genwqe_dev *cd, struct
> genwqe_sgl *sgl,
>   * genwqe_free_sync_sgl() - Free memory for sgl and overlapping pages
>   *
>   * After the DMA transfer has been completed we free the memory for
> - * the sgl and the cached pages. Data is being transfered from cached
> + * the sgl and the cached pages. Data is being transferred from cached
>   * pages into user-space buffers.
>   */
>  int genwqe_free_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl 
> *sgl)

Looks good to me. Thanks.

Frank

Acked-by: Frank Haverkamp <haver@linux.vnet.ibm.com>

[PATCH 3/3] GenWQE: Adjust 12 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 8 Jan 2018 10:21:25 +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/genwqe/card_utils.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
index 0b466664c908..84408dc69020 100644
--- a/drivers/misc/genwqe/card_utils.c
+++ b/drivers/misc/genwqe/card_utils.c
@@ -58,7 +58,7 @@ int __genwqe_writeq(struct genwqe_dev *cd, u64 byte_offs, u64 val)
 	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
 		return -EIO;
 
-	if (cd->mmio == NULL)
+	if (!cd->mmio)
 		return -EIO;
 
 	if (pci_channel_offline(pci_dev))
@@ -88,7 +88,7 @@ u64 __genwqe_readq(struct genwqe_dev *cd, u64 byte_offs)
 	    (byte_offs == IO_SLC_CFGREG_GFIR))
 		return 0x00000000ffff0000ull;
 
-	if (cd->mmio == NULL)
+	if (!cd->mmio)
 		return 0xffffffffffffffffull;
 
 	return be64_to_cpu((__force __be64)__raw_readq(cd->mmio + byte_offs));
@@ -109,7 +109,7 @@ int __genwqe_writel(struct genwqe_dev *cd, u64 byte_offs, u32 val)
 	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
 		return -EIO;
 
-	if (cd->mmio == NULL)
+	if (!cd->mmio)
 		return -EIO;
 
 	if (pci_channel_offline(pci_dev))
@@ -131,7 +131,7 @@ u32 __genwqe_readl(struct genwqe_dev *cd, u64 byte_offs)
 	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
 		return 0xffffffff;
 
-	if (cd->mmio == NULL)
+	if (!cd->mmio)
 		return 0xffffffff;
 
 	return be32_to_cpu((__force __be32)__raw_readl(cd->mmio + byte_offs));
@@ -227,7 +227,7 @@ void *__genwqe_alloc_consistent(struct genwqe_dev *cd, size_t size,
 void __genwqe_free_consistent(struct genwqe_dev *cd, size_t size,
 			     void *vaddr, dma_addr_t dma_handle)
 {
-	if (vaddr == NULL)
+	if (!vaddr)
 		return;
 
 	dma_free_coherent(&cd->pci_dev->dev, size, vaddr, dma_handle);
@@ -323,7 +323,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
 
 	sgl->sgl = __genwqe_alloc_consistent(cd, sgl->sgl_size,
 					     &sgl->sgl_dma_addr);
-	if (sgl->sgl == NULL) {
+	if (!sgl->sgl) {
 		dev_err(&pci_dev->dev,
 			"[%s] err: no memory available!\n", __func__);
 		return -ENOMEM;
@@ -333,7 +333,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
 	if ((sgl->fpage_size != 0) && (sgl->fpage_size != PAGE_SIZE)) {
 		sgl->fpage = __genwqe_alloc_consistent(cd, PAGE_SIZE,
 						       &sgl->fpage_dma_addr);
-		if (sgl->fpage == NULL)
+		if (!sgl->fpage)
 			goto err_out;
 
 		/* Sync with user memory */
@@ -346,7 +346,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
 	if (sgl->lpage_size != 0) {
 		sgl->lpage = __genwqe_alloc_consistent(cd, PAGE_SIZE,
 						       &sgl->lpage_dma_addr);
-		if (sgl->lpage == NULL)
+		if (!sgl->lpage)
 			goto err_out1;
 
 		/* Sync with user memory */
@@ -406,15 +406,12 @@ int genwqe_setup_sgl(struct genwqe_dev *cd, struct genwqe_sgl *sgl,
 			/* DMA mapping for requested page, offs, size */
 			size_to_map = min(size, PAGE_SIZE - map_offs);
 
-			if ((p == 0) && (sgl->fpage != NULL)) {
+			if (p == 0 && sgl->fpage)
 				daddr = sgl->fpage_dma_addr + map_offs;
-
-			} else if ((p == sgl->nr_pages - 1) &&
-				   (sgl->lpage != NULL)) {
+			else if ((p == sgl->nr_pages - 1) && sgl->lpage)
 				daddr = sgl->lpage_dma_addr;
-			} else {
+			else
 				daddr = dma_list[p] + map_offs;
-			}
 
 			size -= size_to_map;
 			map_offs = 0;
@@ -538,7 +535,7 @@ static int genwqe_free_user_pages(struct page **page_list,
 	unsigned int i;
 
 	for (i = 0; i < nr_pages; i++) {
-		if (page_list[i] != NULL) {
+		if (page_list[i]) {
 			if (dirty)
 				set_page_dirty_lock(page_list[i]);
 			put_page(page_list[i]);
@@ -577,7 +574,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
 	unsigned long data, offs;
 	struct pci_dev *pci_dev = cd->pci_dev;
 
-	if ((uaddr == NULL) || (size == 0)) {
+	if (!uaddr || size == 0) {
 		m->size = 0;	/* mark unused and not added */
 		return -EINVAL;
 	}
-- 
2.15.1

Re: [PATCH 3/3] GenWQE: Adjust 12 checks for null pointers

[haver]

Hi Markus,

On 2018-01-08 10:43, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 Jan 2018 10:21:25 +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/genwqe/card_utils.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/misc/genwqe/card_utils.c 
> b/drivers/misc/genwqe/card_utils.c
> index 0b466664c908..84408dc69020 100644
> --- a/drivers/misc/genwqe/card_utils.c
> +++ b/drivers/misc/genwqe/card_utils.c
> @@ -58,7 +58,7 @@ int __genwqe_writeq(struct genwqe_dev *cd, u64
> byte_offs, u64 val)
>  	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
>  		return -EIO;
> 
> -	if (cd->mmio == NULL)
> +	if (!cd->mmio)
>  		return -EIO;
> 
>  	if (pci_channel_offline(pci_dev))
> @@ -88,7 +88,7 @@ u64 __genwqe_readq(struct genwqe_dev *cd, u64 
> byte_offs)
>  	    (byte_offs == IO_SLC_CFGREG_GFIR))
>  		return 0x00000000ffff0000ull;
> 
> -	if (cd->mmio == NULL)
> +	if (!cd->mmio)
>  		return 0xffffffffffffffffull;
> 
>  	return be64_to_cpu((__force __be64)__raw_readq(cd->mmio + 
> byte_offs));
> @@ -109,7 +109,7 @@ int __genwqe_writel(struct genwqe_dev *cd, u64
> byte_offs, u32 val)
>  	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
>  		return -EIO;
> 
> -	if (cd->mmio == NULL)
> +	if (!cd->mmio)
>  		return -EIO;
> 
>  	if (pci_channel_offline(pci_dev))
> @@ -131,7 +131,7 @@ u32 __genwqe_readl(struct genwqe_dev *cd, u64 
> byte_offs)
>  	if (cd->err_inject & GENWQE_INJECT_HARDWARE_FAILURE)
>  		return 0xffffffff;
> 
> -	if (cd->mmio == NULL)
> +	if (!cd->mmio)
>  		return 0xffffffff;
> 
>  	return be32_to_cpu((__force __be32)__raw_readl(cd->mmio + 
> byte_offs));
> @@ -227,7 +227,7 @@ void *__genwqe_alloc_consistent(struct genwqe_dev
> *cd, size_t size,
>  void __genwqe_free_consistent(struct genwqe_dev *cd, size_t size,
>  			     void *vaddr, dma_addr_t dma_handle)
>  {
> -	if (vaddr == NULL)
> +	if (!vaddr)
>  		return;
> 
>  	dma_free_coherent(&cd->pci_dev->dev, size, vaddr, dma_handle);
> @@ -323,7 +323,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
> struct genwqe_sgl *sgl,
> 
>  	sgl->sgl = __genwqe_alloc_consistent(cd, sgl->sgl_size,
>  					     &sgl->sgl_dma_addr);
> -	if (sgl->sgl == NULL) {
> +	if (!sgl->sgl) {
>  		dev_err(&pci_dev->dev,
>  			"[%s] err: no memory available!\n", __func__);
>  		return -ENOMEM;
> @@ -333,7 +333,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
> struct genwqe_sgl *sgl,
>  	if ((sgl->fpage_size != 0) && (sgl->fpage_size != PAGE_SIZE)) {
>  		sgl->fpage = __genwqe_alloc_consistent(cd, PAGE_SIZE,
>  						       &sgl->fpage_dma_addr);
> -		if (sgl->fpage == NULL)
> +		if (!sgl->fpage)
>  			goto err_out;
> 
>  		/* Sync with user memory */
> @@ -346,7 +346,7 @@ int genwqe_alloc_sync_sgl(struct genwqe_dev *cd,
> struct genwqe_sgl *sgl,
>  	if (sgl->lpage_size != 0) {
>  		sgl->lpage = __genwqe_alloc_consistent(cd, PAGE_SIZE,
>  						       &sgl->lpage_dma_addr);
> -		if (sgl->lpage == NULL)
> +		if (!sgl->lpage)
>  			goto err_out1;
> 
>  		/* Sync with user memory */
> @@ -406,15 +406,12 @@ int genwqe_setup_sgl(struct genwqe_dev *cd,
> struct genwqe_sgl *sgl,
>  			/* DMA mapping for requested page, offs, size */
>  			size_to_map = min(size, PAGE_SIZE - map_offs);
> 
> -			if ((p == 0) && (sgl->fpage != NULL)) {
> +			if (p == 0 && sgl->fpage)
>  				daddr = sgl->fpage_dma_addr + map_offs;
> -
> -			} else if ((p == sgl->nr_pages - 1) &&
> -				   (sgl->lpage != NULL)) {
> +			else if ((p == sgl->nr_pages - 1) && sgl->lpage)
>  				daddr = sgl->lpage_dma_addr;
> -			} else {
> +			else
>  				daddr = dma_list[p] + map_offs;
> -			}
> 
>  			size -= size_to_map;
>  			map_offs = 0;
> @@ -538,7 +535,7 @@ static int genwqe_free_user_pages(struct page 
> **page_list,
>  	unsigned int i;
> 
>  	for (i = 0; i < nr_pages; i++) {
> -		if (page_list[i] != NULL) {
> +		if (page_list[i]) {
>  			if (dirty)
>  				set_page_dirty_lock(page_list[i]);
>  			put_page(page_list[i]);
> @@ -577,7 +574,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct
> dma_mapping *m, void *uaddr,
>  	unsigned long data, offs;
>  	struct pci_dev *pci_dev = cd->pci_dev;
> 
> -	if ((uaddr == NULL) || (size == 0)) {
> +	if (!uaddr || size == 0) {
>  		m->size = 0;	/* mark unused and not added */
>  		return -EINVAL;
>  	}

I personally like the explicit compare (ptr != NULL) more than the !ptr 
notation.
When was the checkpatch.pl script modified to suggest the latter 
notation?
Is there any advantage other than the shorter notation?

Regards

Frank

Re: [3/3] GenWQE: Adjust 12 checks for null pointers

[SF Markus Elfring]

> I personally like the explicit compare (ptr != NULL) more than the !ptr notation.

Coding style aspects can evolve, can't they?


> When was the checkpatch.pl script modified to suggest the latter notation?

Would you like to take another look at the software update “checkpatch: add
--strict "pointer comparison to NULL" test” from 2014-12-10?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b75ac618df751b927469ddbca63cf151a62f0f9d


> Is there any advantage other than the shorter notation?

* Do you eventually care for an influence on the run time characteristics
  for the compilation of this software module?

* How do you think about to reduce the dependency on a special preprocessor symbol?

Regards,
Markus