How to fix CHECK warning: testing a 'safe expression'

DISCUSSION Murali Karicheri 4 participants 9 messages Dec 16, 2014 – Jan 2, 2015

Subsystems (4)

Networking Network drivers PCI Sparse checker

lore

Share & tools

lore

Murali Karicheri

author

2014-12-16 18:22 UTC #0 lore raw

netdev maintainers,

I got a comment to address CHECK warning and wondering how to address 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)

where foo is defined as

struct foo_type *foo;

The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?

I have tried doing a grep on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using

$ grep -r "warning: testing a 'safe expression" * | wc -l
276

1) Can someone explain what this warning means?

2) Is it acceptable to post patches to netdev list with this warning?

3) if not, how this is expected to be fixed? Any example usage to fix this warning will be helpful.

Thanks in advance for

Signature · 2 lines

Murali Karicheri
Linux Kernel, Texas Instruments

Murali Karicheri

author

2014-12-17 16:18 UTC #1 lore raw

How to fix CHECK warning: testing a 'safe expression' ?

I am trying to address CHECK warnings in my driver and wondering how to resolve 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)

where foo is defined as

struct foo_type *foo;

The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?

I have tried a grep to check on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using

$ grep -r "warning: testing a 'safe expression" * | wc -l
276

Can someone help me undestand what this means and how to fix the same?

I run folling for CHECK

make C=2 CF="-D__CHECK_ENDIAN__" CHECK=sparse

Thanks in advance for your suggestions.

Signature · 6 lines

Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bjorn Helgaas

maintainer

2014-12-17 18:49 UTC #2 lore raw

Re: How to fix CHECK warning: testing a 'safe expression' ?

[+cc Christopher, linux-kernel (this doesn't sound like a PCI-specific question)]

Quoted 15 lines · Murali Karicheri

I am trying to address CHECK warnings in my driver and wondering how to resolve 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)
where foo is defined as
struct foo_type *foo;
The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?
I have tried a grep to check on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using

Quoted 8 lines

$ grep -r "warning: testing a 'safe expression" * | wc -l
276
Can someone help me undestand what this means and how to fix the same?
I run folling for CHECK
make C=2 CF="-D__CHECK_ENDIAN__" CHECK=sparse

If you can share your actual code or point to it in the tree, you might get more specific help.

Bjorn

Murali Karicheri

author

2014-12-17 22:35 UTC #3 lore raw

Re: How to fix CHECK warning: testing a 'safe expression' ?

Quoted 24 lines · Bjorn Helgaas

[+cc Christopher, linux-kernel (this doesn't sound like a PCI-specific question)]
Quoted 20 lines · Murali Karicheri
I am trying to address CHECK warnings in my driver and wondering how to resolve 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)
where foo is defined as
struct foo_type *foo;
The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?
I have tried a grep to check on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using
$ grep -r "warning: testing a 'safe expression" * | wc -l
276
Can someone help me undestand what this means and how to fix the same?

Quoted 8 lines

Quoted 3 lines
I run folling for CHECK
make C=2 CF="-D__CHECK_ENDIAN__" CHECK=sparse
If you can share your actual code or point to it in the tree, you might get more specific help.
Bjorn

For example I get

CHECK drivers/pci/host/pci-keystone.c
include/linux/err.h:40:16: warning: testing a 'safe expression'

Actually it is for IS_ERR_OR_NULL() call in the file which is the only one call in the file.

Signature · 2 lines

Murali Karicheri
Linux Kernel, Texas Instruments

Murali Karicheri

author

2014-12-17 22:37 UTC #4 lore raw

Re: How to fix CHECK warning: testing a 'safe expression' ?

Quoted 36 lines · Murali Karicheri

Quoted 26 lines · Bjorn Helgaas
[+cc Christopher, linux-kernel (this doesn't sound like a PCI-specific question)]
Quoted 21 lines · Murali Karicheri
I am trying to address CHECK warnings in my driver and wondering how to resolve 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)
where foo is defined as
struct foo_type *foo;
The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?
I have tried a grep to check on the current usage of IS_ERR_OR_NULL() and
found 276 of them causes this warning in the v3.18 version of the kernel that I am using
$ grep -r "warning: testing a 'safe expression" * | wc -l
276
Can someone help me undestand what this means and how to fix the same?
Quoted 8 lines
Quoted 3 lines
I run folling for CHECK
make C=2 CF="-D__CHECK_ENDIAN__" CHECK=sparse
If you can share your actual code or point to it in the tree, you might get more specific help.
Bjorn

Quoted 8 lines

For example I get
CHECK drivers/pci/host/pci-keystone.c include/linux/err.h:40:16: warning: testing a 'safe expression'
Actually it is for IS_ERR_OR_NULL() call in the file which is the only one call in the file.

Here is the code snippet for your convenience

	struct device *dev = &pdev->dev;
	struct keystone_pcie *ks_pcie;
	struct pcie_port *pp;
	struct resource *res;
	void __iomem *reg_p;
	struct phy *phy;
	int ret = 0;

	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
				GFP_KERNEL);
	if (!ks_pcie) {
		dev_err(dev, "no memory for keystone pcie\n");
		return -ENOMEM;
	}
	pp = &ks_pcie->pp;

	/* initialize SerDes Phy if present */
	phy = devm_phy_get(dev, "pcie-phy");
===>    if (!IS_ERR_OR_NULL(phy)) {
		ret = phy_init(phy);
		if (ret < 0)
			return ret;
	}

Signature · 2 lines

Murali Karicheri
Linux Kernel, Texas Instruments

Christopher Li

maintainer

2014-12-18 02:39 UTC #5 lore raw

Re: How to fix CHECK warning: testing a 'safe expression' ?

Quoted 4 lines · Murali Karicheri

        if (!ks_pcie) {
                dev_err(dev, "no memory for keystone pcie\n");
                return -ENOMEM;
        }

Quoted 9 lines

        pp = &ks_pcie->pp;

        /* initialize SerDes Phy if present */
        phy = devm_phy_get(dev, "pcie-phy");
===>    if (!IS_ERR_OR_NULL(phy)) {
                ret = phy_init(phy);
                if (ret < 0)
                        return ret;

Hi,

Do you have a smaller stand alone test case which I can reproduce with sparse?

Thanks

Chris

Murali Karicheri

author

2014-12-18 17:45 UTC #6 lore raw

Re: How to fix CHECK warning: testing a 'safe expression' ?

Quoted 15 lines · Christopher Li

Quoted 4 lines · Murali Karicheri

         if (!ks_pcie) {
                 dev_err(dev, "no memory for keystone pcie\n");
                 return -ENOMEM;
         }

Quoted 9 lines

         pp =&ks_pcie->pp;

         /* initialize SerDes Phy if present */
         phy = devm_phy_get(dev, "pcie-phy");
===>     if (!IS_ERR_OR_NULL(phy)) {
                 ret = phy_init(phy);
                 if (ret<  0)
                         return ret;

Quoted 7 lines

Hi,
Do you have a smaller stand alone test case which I can reproduce with sparse?
Thanks
Chris

Thanks Chris for responding.

You could add this piece of code for testing

struct foo {
	int x;
	int y;
};

static struct foo *func1(struct device *dev)

{
	struct foo *fp;

	fp = devm_kzalloc(dev, sizeof(*fp), GFP_KERNEL);
	if (fp == NULL)
		return ERR_PTR(-ENOMEM);
	return fp;
}

And in one of your module in kernel add

	struct foo *fp;

	if (IS_ERR_OR_NULL(fp))
		return -ENOMEM;

and do

make C=2 CF="-D__CHECK_ENDIAN__" CHECK=sparse

You will see the CHECK warning against your module as

include/linux/err.h:40:16: warning: testing a 'safe expression'

Hope this helps.

Signature · 2 lines

Murali Karicheri
Linux Kernel, Texas Instruments

Murali Karicheri

author

2015-01-02 14:51 UTC #7 lore raw

In reply to Murali Karicheri

+ Josh

Quoted 28 lines · Murali Karicheri

netdev maintainers,
I got a comment to address CHECK warning and wondering how to address 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)
where foo is defined as
struct foo_type *foo;
The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?
I have tried doing a grep on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using
$ grep -r "warning: testing a 'safe expression" * | wc -l
276
1) Can someone explain what this warning means?
2) Is it acceptable to post patches to netdev list with this warning?
3) if not, how this is expected to be fixed? Any example usage to fix this warning will be helpful.
Thanks in advance for

Signature · 2 lines

Murali Karicheri
Linux Kernel, Texas Instruments

Josh Triplett

2015-01-02 22:20 UTC #8 lore raw

+linux-sparse

Quoted 34 lines · Murali Karicheri

Quoted 28 lines · Murali Karicheri
netdev maintainers,
I got a comment to address CHECK warning and wondering how to address 'warning: testing a 'safe expression' which appears when using IS_ERR_OR_NULL(foo)
where foo is defined as
struct foo_type *foo;
The foo get assigned only NULL or ERR_PTR(error code). So I believe the usage is correct. But then how do I make the CHECK happy of its usage?
I have tried doing a grep on the current usage of IS_ERR_OR_NULL() and found 276 of them causes this warning in the v3.18 version of the kernel that I am using
$ grep -r "warning: testing a 'safe expression" * | wc -l
276
1) Can someone explain what this warning means?
2) Is it acceptable to post patches to netdev list with this warning?
3) if not, how this is expected to be fixed? Any example usage to fix this warning will be helpful.
Thanks in advance for
Signature · 2 lines
Murali Karicheri
Linux Kernel, Texas Instruments