[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
Thanks to David, I will submit v3 patch to indicate correct Fixes. > -Original Message- > From: David Marchand [mailto:david.marchand at 6wind.com] > Sent: Wednesday, November 16, 2016 7:24 PM > To: Dai, Wei > Cc: dev at dpdk.org; Burakov, Anatoly > 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 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 v2] eal/linuxapp: fix return value check of mknod()
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 ; Dai, Wei ; > dev at dpdk.org; Burakov, Anatoly ; > david.marchand at 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 at dpdk.org] On Behalf Of Wei Dai > >> Sent: Wednesday, November 16, 2016 10:41 AM > >> To: dev at dpdk.org; Burakov, Anatoly; david.marchand at 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 > >> --- > >> 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. > >
[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
Hello Wei, On Wed, Nov 16, 2016 at 3:40 AM, Wei Dai 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 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 at dpdk.org] On Behalf Of Wei Dai >> Sent: Wednesday, November 16, 2016 10:41 AM >> To: dev at dpdk.org; Burakov, Anatoly; david.marchand at 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 >> --- >> 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.
[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 --- 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
[dpdk-dev] [PATCH v2] eal/linuxapp: fix return value check of mknod()
Hi Wei, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Dai > Sent: Wednesday, November 16, 2016 10:41 AM > To: dev at dpdk.org; Burakov, Anatoly; david.marchand at 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 > --- > 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)