* [PATCH] ptp: fix an IS_ERR() vs NULL check
@ 2018-11-30 12:58 Dan Carpenter
2018-11-30 16:32 ` Richard Cochran
0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2018-11-30 12:58 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, linux-kernel, kernel-janitors
The pps_register_source() function doesn't return NULL, it returns
error pointers.
Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/ptp/ptp_clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 8a81eecc0ecd..48f3594a7458 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
pps.mode = PTP_PPS_MODE;
pps.owner = info->owner;
ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
- if (!ptp->pps_source) {
- err = -EINVAL;
+ if (IS_ERR(ptp->pps_source)) {
+ err = PTR_ERR(ptp->pps_source);
pr_err("failed to register pps source\n");
goto no_pps;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] ptp: fix an IS_ERR() vs NULL check
2018-11-30 12:58 [PATCH] ptp: fix an IS_ERR() vs NULL check Dan Carpenter
@ 2018-11-30 16:32 ` Richard Cochran
2018-12-03 10:04 ` Dan Carpenter
[not found] ` <20181203105506.GA21127@unbuntlaptop>
0 siblings, 2 replies; 12+ messages in thread
From: Richard Cochran @ 2018-11-30 16:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, linux-kernel, kernel-janitors
On Fri, Nov 30, 2018 at 03:58:34PM +0300, Dan Carpenter wrote:
> The pps_register_source() function doesn't return NULL, it returns
> error pointers.
It keeps a local variable for errno, but then it returns NULL.
But this is about to change with this recent patch:
26.Nov'18 YueHaibing [PATCH -next] pps: using ERR_PTR instead of NULL while pps_register_source fails
It hasn't been merged yet AFAICT, but gregkh said he take it.
Thanks,
Richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ptp: fix an IS_ERR() vs NULL check
2018-11-30 16:32 ` Richard Cochran
@ 2018-12-03 10:04 ` Dan Carpenter
[not found] ` <20181203105506.GA21127@unbuntlaptop>
1 sibling, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-12-03 10:04 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, linux-kernel, kernel-janitors
On Fri, Nov 30, 2018 at 08:32:52AM -0800, Richard Cochran wrote:
> On Fri, Nov 30, 2018 at 03:58:34PM +0300, Dan Carpenter wrote:
> > The pps_register_source() function doesn't return NULL, it returns
> > error pointers.
>
> It keeps a local variable for errno, but then it returns NULL.
> But this is about to change with this recent patch:
>
> 26.Nov'18 YueHaibing [PATCH -next] pps: using ERR_PTR instead of NULL while pps_register_source fails
>
> It hasn't been merged yet AFAICT, but gregkh said he take it.
Oh, yeah. That patch has actually been applied, but we didn't update
the caller. I should have made the Fixes tag point to that YueHaibing's
patch.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
[not found] ` <20181203105506.GA21127@unbuntlaptop>
@ 2018-12-04 4:42 ` Richard Cochran
2018-12-04 7:00 ` Dan Carpenter
0 siblings, 1 reply; 12+ messages in thread
From: Richard Cochran @ 2018-12-04 4:42 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, linux-kernel, kernel-janitors
On Mon, Dec 03, 2018 at 01:55:06PM +0300, Dan Carpenter wrote:
> We recently modified pps_register_source() to return error pointers
> instead of NULL but this check wasn't updated.
But it *was* updated!
> Fixes: 3b1ad360acad ("pps: using ERR_PTR instead of NULL while pps_register_source fails")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Use the correct Fixes tag. Add Greg to the CC list, because he
> is maintaining this driver.
What driver? (I am maintaining drivers/ptp/ptp_clock.c)
> drivers/ptp/ptp_clock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 8a81eecc0ecd..48f3594a7458 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> pps.mode = PTP_PPS_MODE;
> pps.owner = info->owner;
> ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
> - if (!ptp->pps_source) {
> - err = -EINVAL;
> + if (IS_ERR(ptp->pps_source)) {
> + err = PTR_ERR(ptp->pps_source);
> pr_err("failed to register pps source\n");
> goto no_pps;
> }
YueHaibing's patch has the following hunk. It really is already
there. I don't see it yet on Linus' master branch, but the patch
should update drivers/pps/kapi.c and the callers all in one commit.
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 8a81eec..48f3594 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
pps.mode = PTP_PPS_MODE;
pps.owner = info->owner;
ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
- if (!ptp->pps_source) {
- err = -EINVAL;
+ if (IS_ERR(ptp->pps_source)) {
+ err = PTR_ERR(ptp->pps_source);
pr_err("failed to register pps source\n");
goto no_pps;
}
Thanks,
Richard
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-04 4:42 ` [PATCH v2] " Richard Cochran
@ 2018-12-04 7:00 ` Dan Carpenter
2018-12-04 10:54 ` Greg Kroah-Hartman
2018-12-04 14:55 ` Richard Cochran
0 siblings, 2 replies; 12+ messages in thread
From: Dan Carpenter @ 2018-12-04 7:00 UTC (permalink / raw)
To: Richard Cochran, Greg Kroah-Hartman
Cc: netdev, linux-kernel, kernel-janitors, YueHaibing
Hi Greg,
I meant to CC you but I screwed up and added you to the From header
instead... :(
Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
while pps_register_source fails")? You're not listed as a maintainer so
I wouldn't have known to CC you.
The back story is that the last chunk which changes drivers/ptp/ptp_clock.c
was dropped by mistake and it's not there in linux-next. I wrote a
patch which adds it, but everyone is super confused now... Here is
YueHaibing's original patch btw, for reference.
https://www.spinics.net/lists/netdev/msg535929.html
regards,
dan carpenter
On Mon, Dec 03, 2018 at 08:42:57PM -0800, Richard Cochran wrote:
> On Mon, Dec 03, 2018 at 01:55:06PM +0300, Dan Carpenter wrote:
> > We recently modified pps_register_source() to return error pointers
> > instead of NULL but this check wasn't updated.
>
> But it *was* updated!
>
> > Fixes: 3b1ad360acad ("pps: using ERR_PTR instead of NULL while pps_register_source fails")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: Use the correct Fixes tag. Add Greg to the CC list, because he
> > is maintaining this driver.
>
> What driver? (I am maintaining drivers/ptp/ptp_clock.c)
>
> > drivers/ptp/ptp_clock.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> > index 8a81eecc0ecd..48f3594a7458 100644
> > --- a/drivers/ptp/ptp_clock.c
> > +++ b/drivers/ptp/ptp_clock.c
> > @@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> > pps.mode = PTP_PPS_MODE;
> > pps.owner = info->owner;
> > ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
> > - if (!ptp->pps_source) {
> > - err = -EINVAL;
> > + if (IS_ERR(ptp->pps_source)) {
> > + err = PTR_ERR(ptp->pps_source);
> > pr_err("failed to register pps source\n");
> > goto no_pps;
> > }
>
> YueHaibing's patch has the following hunk. It really is already
> there. I don't see it yet on Linus' master branch, but the patch
> should update drivers/pps/kapi.c and the callers all in one commit.
>
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 8a81eec..48f3594 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> pps.mode = PTP_PPS_MODE;
> pps.owner = info->owner;
> ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
> - if (!ptp->pps_source) {
> - err = -EINVAL;
> + if (IS_ERR(ptp->pps_source)) {
> + err = PTR_ERR(ptp->pps_source);
> pr_err("failed to register pps source\n");
> goto no_pps;
> }
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-04 7:00 ` Dan Carpenter
@ 2018-12-04 10:54 ` Greg Kroah-Hartman
2018-12-04 14:55 ` Richard Cochran
1 sibling, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-04 10:54 UTC (permalink / raw)
To: Dan Carpenter
Cc: Richard Cochran, netdev, linux-kernel, kernel-janitors, YueHaibing
On Tue, Dec 04, 2018 at 10:00:33AM +0300, Dan Carpenter wrote:
> Hi Greg,
>
> I meant to CC you but I screwed up and added you to the From header
> instead... :(
>
> Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
> while pps_register_source fails")? You're not listed as a maintainer so
> I wouldn't have known to CC you.
The maintainer told me to do so, see this email thread:
https://lore.kernel.org/lkml/44edc27b-e679-24e8-aef1-fe5b7570f982@enneenne.com/
> The back story is that the last chunk which changes drivers/ptp/ptp_clock.c
> was dropped by mistake and it's not there in linux-next. I wrote a
> patch which adds it, but everyone is super confused now... Here is
> YueHaibing's original patch btw, for reference.
> https://www.spinics.net/lists/netdev/msg535929.html
If you want me to take a fixup patch for this, I will be glad to. I can
also drop it from my tree as well, just let me know.
Sorry for the confusion.
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-04 7:00 ` Dan Carpenter
2018-12-04 10:54 ` Greg Kroah-Hartman
@ 2018-12-04 14:55 ` Richard Cochran
2018-12-04 15:10 ` Dan Carpenter
1 sibling, 1 reply; 12+ messages in thread
From: Richard Cochran @ 2018-12-04 14:55 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, netdev, linux-kernel, kernel-janitors, YueHaibing
On Tue, Dec 04, 2018 at 10:00:33AM +0300, Dan Carpenter wrote:
>
> Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
> while pps_register_source fails")? You're not listed as a maintainer so
> I wouldn't have known to CC you.
I'm confused. Where is that commit?
$ git show 3b1ad360acad
fatal: ambiguous argument '3b1ad360acad': unknown revision or path not in the working tree.
I'm pulling from:
torvalds git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git (fetch)
stable git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git (fetch)
(and also from net and net-next)
Thanks,
Richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-04 14:55 ` Richard Cochran
@ 2018-12-04 15:10 ` Dan Carpenter
2018-12-06 12:38 ` Richard Cochran
0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2018-12-04 15:10 UTC (permalink / raw)
To: Richard Cochran
Cc: Greg Kroah-Hartman, netdev, linux-kernel, kernel-janitors, YueHaibing
On Tue, Dec 04, 2018 at 06:55:38AM -0800, Richard Cochran wrote:
> On Tue, Dec 04, 2018 at 10:00:33AM +0300, Dan Carpenter wrote:
> >
> > Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
> > while pps_register_source fails")? You're not listed as a maintainer so
> > I wouldn't have known to CC you.
>
> I'm confused. Where is that commit?
>
> $ git show 3b1ad360acad
> fatal: ambiguous argument '3b1ad360acad': unknown revision or path not in the working tree.
>
> I'm pulling from:
>
> torvalds git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git (fetch)
> stable git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git (fetch)
>
> (and also from net and net-next)
>
I'm on linux-next but originally it came from the char-misc-next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next
regards,
dan carpenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-04 15:10 ` Dan Carpenter
@ 2018-12-06 12:38 ` Richard Cochran
2018-12-06 13:57 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Richard Cochran @ 2018-12-06 12:38 UTC (permalink / raw)
To: Dan Carpenter
Cc: Greg Kroah-Hartman, netdev, linux-kernel, kernel-janitors, YueHaibing
Greg,
On Tue, Dec 04, 2018 at 06:10:49PM +0300, Dan Carpenter wrote:
> On Tue, Dec 04, 2018 at 06:55:38AM -0800, Richard Cochran wrote:
> > On Tue, Dec 04, 2018 at 10:00:33AM +0300, Dan Carpenter wrote:
> > >
> > > Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
> > > while pps_register_source fails")? You're not listed as a maintainer so
> > > I wouldn't have known to CC you.
> >
> > I'm confused. Where is that commit?
...
> I'm on linux-next but originally it came from the char-misc-next tree.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next
Commit 3b1ad360acad changes a return value in the PPS core, but it is
missing the hunk for the caller in drivers/ptp/ptp_clock.c. Can that
be amended?
Thanks,
Richard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] ptp: fix an IS_ERR() vs NULL check
2018-12-06 12:38 ` Richard Cochran
@ 2018-12-06 13:57 ` Greg Kroah-Hartman
2018-12-07 6:00 ` [PATCH v3] " Dan Carpenter
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-06 13:57 UTC (permalink / raw)
To: Richard Cochran
Cc: Dan Carpenter, netdev, linux-kernel, kernel-janitors, YueHaibing
On Thu, Dec 06, 2018 at 04:38:43AM -0800, Richard Cochran wrote:
> Greg,
>
> On Tue, Dec 04, 2018 at 06:10:49PM +0300, Dan Carpenter wrote:
> > On Tue, Dec 04, 2018 at 06:55:38AM -0800, Richard Cochran wrote:
> > > On Tue, Dec 04, 2018 at 10:00:33AM +0300, Dan Carpenter wrote:
> > > >
> > > > Why did you commit 3b1ad360acad ("pps: using ERR_PTR instead of NULL
> > > > while pps_register_source fails")? You're not listed as a maintainer so
> > > > I wouldn't have known to CC you.
> > >
> > > I'm confused. Where is that commit?
>
> ...
>
> > I'm on linux-next but originally it came from the char-misc-next tree.
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next
>
> Commit 3b1ad360acad changes a return value in the PPS core, but it is
> missing the hunk for the caller in drivers/ptp/ptp_clock.c. Can that
> be amended?
Send me a patch or tell me what I need to revert and I will be glad to
do so :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] ptp: fix an IS_ERR() vs NULL check
2018-12-06 13:57 ` Greg Kroah-Hartman
@ 2018-12-07 6:00 ` Dan Carpenter
2018-12-12 14:17 ` Richard Cochran
0 siblings, 1 reply; 12+ messages in thread
From: Dan Carpenter @ 2018-12-07 6:00 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, kernel-janitors, YueHaibing, Greg Kroah-Hartman
We recently modified pps_register_source() to return error pointers
instead of NULL but it seems like there was a merge issue and part of
the commit was lost. Anyway, the ptp_clock_register() function needs to
be updated to check for IS_ERR() as well.
Fixes: 3b1ad360acad ("pps: using ERR_PTR instead of NULL while pps_register_source fails")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Use the correct Fixes tag.
v3: Add Greg to the CC list for real this time.
drivers/ptp/ptp_clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 8a81eecc0ecd..48f3594a7458 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -265,8 +265,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
pps.mode = PTP_PPS_MODE;
pps.owner = info->owner;
ptp->pps_source = pps_register_source(&pps, PTP_PPS_DEFAULTS);
- if (!ptp->pps_source) {
- err = -EINVAL;
+ if (IS_ERR(ptp->pps_source)) {
+ err = PTR_ERR(ptp->pps_source);
pr_err("failed to register pps source\n");
goto no_pps;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] ptp: fix an IS_ERR() vs NULL check
2018-12-07 6:00 ` [PATCH v3] " Dan Carpenter
@ 2018-12-12 14:17 ` Richard Cochran
0 siblings, 0 replies; 12+ messages in thread
From: Richard Cochran @ 2018-12-12 14:17 UTC (permalink / raw)
To: Dan Carpenter
Cc: netdev, linux-kernel, kernel-janitors, YueHaibing, Greg Kroah-Hartman
On Fri, Dec 07, 2018 at 09:00:46AM +0300, Dan Carpenter wrote:
> We recently modified pps_register_source() to return error pointers
> instead of NULL but it seems like there was a merge issue and part of
> the commit was lost. Anyway, the ptp_clock_register() function needs to
> be updated to check for IS_ERR() as well.
>
> Fixes: 3b1ad360acad ("pps: using ERR_PTR instead of NULL while pps_register_source fails")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Greg,
This patch provides the missing hunk. Please apply it.
Thanks,
Richard
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-12-12 14:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30 12:58 [PATCH] ptp: fix an IS_ERR() vs NULL check Dan Carpenter
2018-11-30 16:32 ` Richard Cochran
2018-12-03 10:04 ` Dan Carpenter
[not found] ` <20181203105506.GA21127@unbuntlaptop>
2018-12-04 4:42 ` [PATCH v2] " Richard Cochran
2018-12-04 7:00 ` Dan Carpenter
2018-12-04 10:54 ` Greg Kroah-Hartman
2018-12-04 14:55 ` Richard Cochran
2018-12-04 15:10 ` Dan Carpenter
2018-12-06 12:38 ` Richard Cochran
2018-12-06 13:57 ` Greg Kroah-Hartman
2018-12-07 6:00 ` [PATCH v3] " Dan Carpenter
2018-12-12 14:17 ` Richard Cochran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).