[dpdk-dev] [PATCH] eal/linuxapp: fix return value check of mknod()

[dpdk-dev] [PATCH] eal/linuxapp: fix return value check of mknod()

[Zhiyong Yang]

From: Wei Dai <wei.dai@intel.com>

In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
The return value of mknod() is ret, not f got by fopen().
So the value of ret should be checked for mknod().

Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 1786b75..3e4ffb5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path, unsigned uio_num)
 	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 	dev = makedev(major, minor);
 	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
-	if (f == NULL) {
+	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "%s(): mknod() failed %s\n",
 			__func__, strerror(errno));
 		return -1;
-- 
2.5.5

[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Wei Dai]

In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
The return value of mknod() is ret, not f got by fopen().
So the value of ret should be checked for mknod().

Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
fix my local git setting and send same patch again to
make merging easier

 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 1786b75..3e4ffb5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path, unsigned uio_num)
 	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 	dev = makedev(major, minor);
 	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
-	if (f == NULL) {
+	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "%s(): mknod() failed %s\n",
 			__func__, strerror(errno));
 		return -1;
-- 
2.5.5

Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Lu, Wenzhuo]

Hi Wei,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> Sent: Wednesday, November 16, 2016 10:41 AM
> To: dev@dpdk.org; Burakov, Anatoly; david.marchand@6wind.com; Dai, Wei
> Subject: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
> 
> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c, The return
> value of mknod() is ret, not f got by fopen().
> So the value of ret should be checked for mknod().
> 
> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
> fix my local git setting and send same patch again to make merging easier
> 
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 1786b75..3e4ffb5 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path,
> unsigned uio_num)
>  	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
>  	dev = makedev(major, minor);
>  	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
> -	if (f == NULL) {
> +	if (ret != 0) {
I think checkpatch will suggest to just use if (ret)

Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Ferruh Yigit]

Hi Wenzhuo,

On 11/16/2016 3:28 AM, Lu, Wenzhuo wrote:
> Hi Wei,
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
>> Sent: Wednesday, November 16, 2016 10:41 AM
>> To: dev@dpdk.org; Burakov, Anatoly; david.marchand@6wind.com; Dai, Wei
>> Subject: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
>>
>> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c, The return
>> value of mknod() is ret, not f got by fopen().
>> So the value of ret should be checked for mknod().
>>
>> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
>>
>> Signed-off-by: Wei Dai <wei.dai@intel.com>
>> ---
>> fix my local git setting and send same patch again to make merging easier
>>
>>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> index 1786b75..3e4ffb5 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> @@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path,
>> unsigned uio_num)
>>  	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
>>  	dev = makedev(major, minor);
>>  	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
>> -	if (f == NULL) {
>> +	if (ret != 0) {
> I think checkpatch will suggest to just use if (ret)

Your are right, default checkpatch.pl complains about this usage (with
--strict option), but:

- According DPDK coding style this usage is preferred (although I
personally prefer kernel one..)

http://dpdk.org/doc/guides/contributing/coding_style.html#null-pointers

"
if (p == NULL) /* Good, compare pointer to NULL */

if (!p) /* Bad, using ! on pointer */
"

- This warning disabled in dpdk scripts/checkpatches.sh by "--ignore
COMPARISON_TO_NULL", so it shouldn't complain.

Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Dai, Wei]

Thanks to Yigit Ferruh and Wenzhuo for your guide.
Several months ago, I download checkpatch.pl and put it in /root/bin/.
In /root/.bash_profile in my server,  there is line :export DPDK_CHECKPATCH_PATH=/root/bin/checkpatch.pl
Before I send this patch, I have run checkpath.sh to check it and it show no error.
./scripts/checkpatch.sh -v v2-0001-eal-*.patch

By search ' != 0', there are many lines in many modules of DPDK.
So I think ' !=0' is OK.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, November 16, 2016 7:04 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Dai, Wei <wei.dai@intel.com>;
> dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>;
> david.marchand@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of
> mknod()
> 
> Hi Wenzhuo,
> 
> On 11/16/2016 3:28 AM, Lu, Wenzhuo wrote:
> > Hi Wei,
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> >> Sent: Wednesday, November 16, 2016 10:41 AM
> >> To: dev@dpdk.org; Burakov, Anatoly; david.marchand@6wind.com; Dai,
> >> Wei
> >> Subject: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check
> >> of mknod()
> >>
> >> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> >> The return value of mknod() is ret, not f got by fopen().
> >> So the value of ret should be checked for mknod().
> >>
> >> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
> >>
> >> Signed-off-by: Wei Dai <wei.dai@intel.com>
> >> ---
> >> fix my local git setting and send same patch again to make merging
> >> easier
> >>
> >>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> index 1786b75..3e4ffb5 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> @@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path,
> >> unsigned uio_num)
> >>  	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
> >>  	dev = makedev(major, minor);
> >>  	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
> >> -	if (f == NULL) {
> >> +	if (ret != 0) {
> > I think checkpatch will suggest to just use if (ret)
> 
> Your are right, default checkpatch.pl complains about this usage (with --strict
> option), but:
> 
> - According DPDK coding style this usage is preferred (although I personally
> prefer kernel one..)
> 
> http://dpdk.org/doc/guides/contributing/coding_style.html#null-pointers
> 
> "
> if (p == NULL) /* Good, compare pointer to NULL */
> 
> if (!p) /* Bad, using ! on pointer */
> "
> 
> - This warning disabled in dpdk scripts/checkpatches.sh by "--ignore
> COMPARISON_TO_NULL", so it shouldn't complain.
> 
> 

Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[David Marchand]

Hello Wei,

On Wed, Nov 16, 2016 at 3:40 AM, Wei Dai <wei.dai@intel.com> wrote:
> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> The return value of mknod() is ret, not f got by fopen().
> So the value of ret should be checked for mknod().
>
> Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")

The commit you are pointing is just moving the code.
I would incriminate f7f97c16048e ("pci: add option --create-uio-dev to
run without hotplug")

The rest looks good to me.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()

[Dai, Wei]

Thanks to David, I will submit v3 patch to indicate correct Fixes.

> -----Original Message-----
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Wednesday, November 16, 2016 7:24 PM
> To: Dai, Wei <wei.dai@intel.com>
> Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: Re: [PATCH v2] eal/linuxapp: fix return value check of mknod()
> 
> Hello Wei,
> 
> On Wed, Nov 16, 2016 at 3:40 AM, Wei Dai <wei.dai@intel.com> wrote:
> > In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> > The return value of mknod() is ret, not f got by fopen().
> > So the value of ret should be checked for mknod().
> >
> > Fixes: 67c536bdad93 ("pci: move uio mapping in a dedicated file")
> 
> The commit you are pointing is just moving the code.
> I would incriminate f7f97c16048e ("pci: add option --create-uio-dev to run
> without hotplug")
> 
> The rest looks good to me.
> 
> 
> --
> David Marchand

[dpdk-dev] [PATCH v3] eal/linuxapp: fix return value check of mknod()

[Wei Dai]

In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
The return value of mknod() is ret, not f got by fopen().
So the value of ret should be checked for mknod().

Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without hotplug")

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
v3:
* correct Fixes: line in git commit message body

v2:
* fix my local git setting and send same patch again to remove
  "From: Wei Dai <wei.dai@intel.com>" in git commit message body
  and make merging easier

 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 1786b75..3e4ffb5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -133,7 +133,7 @@ pci_mknod_uio_dev(const char *sysfs_uio_path, unsigned uio_num)
 	snprintf(filename, sizeof(filename), "/dev/uio%u", uio_num);
 	dev = makedev(major, minor);
 	ret = mknod(filename, S_IFCHR | S_IRUSR | S_IWUSR, dev);
-	if (f == NULL) {
+	if (ret != 0) {
 		RTE_LOG(ERR, EAL, "%s(): mknod() failed %s\n",
 			__func__, strerror(errno));
 		return -1;
-- 
2.5.5

Re: [dpdk-dev] [PATCH v3] eal/linuxapp: fix return value check of mknod()

[Olivier Matz]

Hi Wei,

On Thu, 17 Nov 2016 14:47:15 +0800, Wei Dai <wei.dai@intel.com> wrote:
> In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> The return value of mknod() is ret, not f got by fopen().
> So the value of ret should be checked for mknod().
> 
> Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without
> hotplug")
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Thanks

Re: [dpdk-dev] [PATCH v3] eal/linuxapp: fix return value check of mknod()

[Thomas Monjalon]

2016-12-06 11:44, Olivier Matz:
> Hi Wei,
> 
> On Thu, 17 Nov 2016 14:47:15 +0800, Wei Dai <wei.dai@intel.com> wrote:
> > In function pci_mknod_uio_dev() in lib/librte_eal/eal/eal_pci_uio.c,
> > The return value of mknod() is ret, not f got by fopen().
> > So the value of ret should be checked for mknod().
> > 
> > Fixes: f7f97c16048e ("pci: add option --create-uio-dev to run without
> > hotplug")

CC: stable@dpdk.org

> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Thanks

Applied, thanks