[PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

[PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

[Andy Shevchenko]

Make wwan_create_dev() to return either valid or error pointer,
In some cases it may return NULL. Prevent this by converting
it to the respective error pointer.

Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: rewrote to return error pointer, align callers (Loic)
 drivers/net/wwan/wwan_core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 674a81d79db3..35ece98134c0 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -164,11 +164,14 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
 		goto done_unlock;
 
 	id = ida_alloc(&wwan_dev_ids, GFP_KERNEL);
-	if (id < 0)
+	if (id < 0) {
+		wwandev = ERR_PTR(id);
 		goto done_unlock;
+	}
 
 	wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
 	if (!wwandev) {
+		wwandev = ERR_PTR(-ENOMEM);
 		ida_free(&wwan_dev_ids, id);
 		goto done_unlock;
 	}
@@ -182,7 +185,8 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
 	err = device_register(&wwandev->dev);
 	if (err) {
 		put_device(&wwandev->dev);
-		wwandev = NULL;
+		wwandev = ERR_PTR(err);
+		goto done_unlock;
 	}
 
 done_unlock:
@@ -1014,8 +1018,8 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
 		return -EINVAL;
 
 	wwandev = wwan_create_dev(parent);
-	if (!wwandev)
-		return -ENOMEM;
+	if (IS_ERR(wwandev))
+		return PTR_ERR(wwandev);
 
 	if (WARN_ON(wwandev->ops)) {
 		wwan_remove_dev(wwandev);
-- 
2.30.2

[PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Andy Shevchenko]

ida_alloc_range)) may return other than -ENOMEM error code.
Unshadow it in the wwan_create_port().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
 drivers/net/wwan/wwan_core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 35ece98134c0..d293ab688044 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -359,8 +359,8 @@ struct wwan_port *wwan_create_port(struct device *parent,
 {
 	struct wwan_device *wwandev;
 	struct wwan_port *port;
-	int minor, err = -ENOMEM;
 	char namefmt[0x20];
+	int minor, err;
 
 	if (type > WWAN_PORT_MAX || !ops)
 		return ERR_PTR(-EINVAL);
@@ -374,11 +374,14 @@ struct wwan_port *wwan_create_port(struct device *parent,
 
 	/* A port is exposed as character device, get a minor */
 	minor = ida_alloc_range(&minors, 0, WWAN_MAX_MINORS - 1, GFP_KERNEL);
-	if (minor < 0)
+	if (minor < 0) {
+		err = minor;
 		goto error_wwandev_remove;
+	}
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
 	if (!port) {
+		err = -ENOMEM;
 		ida_free(&minors, minor);
 		goto error_wwandev_remove;
 	}
-- 
2.30.2

Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Sergey Ryazanov]

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> ida_alloc_range)) may return other than -ENOMEM error code.
> Unshadow it in the wwan_create_port().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

A small nitpick, looks like "ida_alloc_range))" in the description is
a typo and should be "ida_alloc_range()". Besides this:

Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Andy Shevchenko]

On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > ida_alloc_range)) may return other than -ENOMEM error code.
> > Unshadow it in the wwan_create_port().
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> A small nitpick, looks like "ida_alloc_range))" in the description is
> a typo and should be "ida_alloc_range()". Besides this:

Shall I resend?

> Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Sergey Ryazanov]

On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
>> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> ida_alloc_range)) may return other than -ENOMEM error code.
>>> Unshadow it in the wwan_create_port().
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> A small nitpick, looks like "ida_alloc_range))" in the description is
>> a typo and should be "ida_alloc_range()". Besides this:
>
> Shall I resend?

Yes, please. And specify the target tree in the subject, please. See
patchwork warning [1, 2]. The first patch is a clear bug fix, so it
should be targeted to the 'net' tree, while the second patch despite
its usefulness could not be considered a bug fix, so it should be
targeted to the 'net-next' tree. Subjects could be like this:

[PATCHv3 net 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()
[PATCHv3 net-next 2/2] wwan: core: Unshadow error code returned by
ida_alloc_range()

Or since the second patch is not depends on the first one and patches
target different trees, patches could be submitted independently:

[PATCHv3 net] wwan: core: Avoid returning NULL from wwan_create_dev()
[PATCHv3 net-next] wwan: core: Unshadow error code returned by ida_alloc_range()




1. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-1-andriy.shevchenko@linux.intel.com/
2. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-2-andriy.shevchenko@linux.intel.com/

-- 
Sergey

Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Andy Shevchenko]

On Fri, Aug 06, 2021 at 11:35:04PM +0300, Sergey Ryazanov wrote:
> On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote:
> >> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
> >> <andriy.shevchenko@linux.intel.com> wrote:
> >>> ida_alloc_range)) may return other than -ENOMEM error code.
> >>> Unshadow it in the wwan_create_port().
> >>>
> >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >>
> >> A small nitpick, looks like "ida_alloc_range))" in the description is
> >> a typo and should be "ida_alloc_range()". Besides this:
> >
> > Shall I resend?
> 
> Yes, please. And specify the target tree in the subject, please. See
> patchwork warning [1, 2]. The first patch is a clear bug fix, so it
> should be targeted to the 'net' tree, while the second patch despite
> its usefulness could not be considered a bug fix, so it should be
> targeted to the 'net-next' tree. Subjects could be like this:
> 
> [PATCHv3 net 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()
> [PATCHv3 net-next 2/2] wwan: core: Unshadow error code returned by
> ida_alloc_range()
> 
> Or since the second patch is not depends on the first one and patches
> target different trees, patches could be submitted independently:
> 
> [PATCHv3 net] wwan: core: Avoid returning NULL from wwan_create_dev()
> [PATCHv3 net-next] wwan: core: Unshadow error code returned by ida_alloc_range()

Split and sent separately, thanks!

> 1. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-1-andriy.shevchenko@linux.intel.com/
> 2. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-2-andriy.shevchenko@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

[Loic Poulain]

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> ida_alloc_range)) may return other than -ENOMEM error code.
> Unshadow it in the wwan_create_port().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

[Sergey Ryazanov]

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Make wwan_create_dev() to return either valid or error pointer,
> In some cases it may return NULL. Prevent this by converting
> it to the respective error pointer.
>
> Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: rewrote to return error pointer, align callers (Loic)

Acked-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Re: [PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

[Loic Poulain]

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Make wwan_create_dev() to return either valid or error pointer,
> In some cases it may return NULL. Prevent this by converting
> it to the respective error pointer.
>
> Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>