* [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-17 13:37 ` Khalid Masum
0 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-17 13:37 UTC (permalink / raw)
To: linux-kernel-mentees, linux-kernel
Cc: Shuah Khan, Pavel Skripkin, Marcel Holtmann, Johan Hedberg,
Luiz Augusto von Dentz, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-bluetooth, netdev,
Khalid Masum
Failure of kzalloc to allocate memory is not reported. Return Error
pointer to ENOMEM if memory allocation fails. This will increase
readability and will make the function easier to use in future.
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
---
drivers/bluetooth/btusb.c | 4 ++--
net/bluetooth/hci_core.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index e25fcd49db70..3407762b3b15 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3692,8 +3692,8 @@ static int btusb_probe(struct usb_interface *intf,
data->recv_acl = hci_recv_frame;
hdev = hci_alloc_dev_priv(priv_size);
- if (!hdev)
- return -ENOMEM;
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
hdev->bus = HCI_USB;
hci_set_drvdata(hdev, data);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a0f99baafd35..ea50767e02bf 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
hdev = kzalloc(alloc_size, GFP_KERNEL);
if (!hdev)
- return NULL;
+ return ERR_PTR(-ENOMEM);
hdev->pkt_type = (HCI_DM1 | HCI_DH1 | HCI_HV1);
hdev->esco_type = (ESCO_HV1);
--
2.36.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-17 13:37 ` Khalid Masum
0 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-17 13:37 UTC (permalink / raw)
To: linux-kernel-mentees, linux-kernel
Cc: Johan Hedberg, Pavel Skripkin, Marcel Holtmann, linux-bluetooth,
Eric Dumazet, netdev, Luiz Augusto von Dentz, Jakub Kicinski,
Paolo Abeni, David S . Miller
Failure of kzalloc to allocate memory is not reported. Return Error
pointer to ENOMEM if memory allocation fails. This will increase
readability and will make the function easier to use in future.
Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
---
drivers/bluetooth/btusb.c | 4 ++--
net/bluetooth/hci_core.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index e25fcd49db70..3407762b3b15 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3692,8 +3692,8 @@ static int btusb_probe(struct usb_interface *intf,
data->recv_acl = hci_recv_frame;
hdev = hci_alloc_dev_priv(priv_size);
- if (!hdev)
- return -ENOMEM;
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
hdev->bus = HCI_USB;
hci_set_drvdata(hdev, data);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a0f99baafd35..ea50767e02bf 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
hdev = kzalloc(alloc_size, GFP_KERNEL);
if (!hdev)
- return NULL;
+ return ERR_PTR(-ENOMEM);
hdev->pkt_type = (HCI_DM1 | HCI_DH1 | HCI_HV1);
hdev->esco_type = (ESCO_HV1);
--
2.36.1
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: Bluetooth: hci_core: Use ERR_PTR instead of NULL
2022-07-17 13:37 ` Khalid Masum
(?)
@ 2022-07-17 14:04 ` bluez.test.bot
-1 siblings, 0 replies; 11+ messages in thread
From: bluez.test.bot @ 2022-07-17 14:04 UTC (permalink / raw)
To: linux-bluetooth, khalid.masum.92
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=660435
---Test result---
Test Summary:
CheckPatch PASS 1.44 seconds
GitLint PASS 0.84 seconds
SubjectPrefix PASS 0.65 seconds
BuildKernel PASS 31.55 seconds
BuildKernel32 PASS 27.58 seconds
Incremental Build with patchesPASS 37.85 seconds
TestRunner: Setup PASS 468.67 seconds
TestRunner: l2cap-tester PASS 16.39 seconds
TestRunner: bnep-tester PASS 5.54 seconds
TestRunner: mgmt-tester PASS 94.94 seconds
TestRunner: rfcomm-tester PASS 8.95 seconds
TestRunner: sco-tester PASS 8.67 seconds
TestRunner: smp-tester PASS 8.79 seconds
TestRunner: userchan-tester PASS 5.78 seconds
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
2022-07-17 13:37 ` Khalid Masum
@ 2022-07-17 16:17 ` Pavel Skripkin
-1 siblings, 0 replies; 11+ messages in thread
From: Pavel Skripkin @ 2022-07-17 16:17 UTC (permalink / raw)
To: Khalid Masum, linux-kernel-mentees, linux-kernel
Cc: Shuah Khan, Marcel Holtmann, Johan Hedberg,
Luiz Augusto von Dentz, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-bluetooth, netdev
Hi Khalid,
Khalid Masum <khalid.masum.92@gmail.com> says:
> Failure of kzalloc to allocate memory is not reported. Return Error
> pointer to ENOMEM if memory allocation fails. This will increase
> readability and will make the function easier to use in future.
>
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> ---
[snip]
> index a0f99baafd35..ea50767e02bf 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
>
> hdev = kzalloc(alloc_size, GFP_KERNEL);
> if (!hdev)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
This will break all callers of hci_alloc_dev(). All callers expect NULL
in case of an error, so you will leave them with wrong pointer.
Also, allocation functionS return an error only in case of ENOMEM, so
initial code is fine, IMO
Thanks,
--Pavel Skripkin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-17 16:17 ` Pavel Skripkin
0 siblings, 0 replies; 11+ messages in thread
From: Pavel Skripkin @ 2022-07-17 16:17 UTC (permalink / raw)
To: Khalid Masum, linux-kernel-mentees, linux-kernel
Cc: Johan Hedberg, netdev, Marcel Holtmann, linux-bluetooth,
Eric Dumazet, Luiz Augusto von Dentz, Jakub Kicinski,
Paolo Abeni, David S . Miller
Hi Khalid,
Khalid Masum <khalid.masum.92@gmail.com> says:
> Failure of kzalloc to allocate memory is not reported. Return Error
> pointer to ENOMEM if memory allocation fails. This will increase
> readability and will make the function easier to use in future.
>
> Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> ---
[snip]
> index a0f99baafd35..ea50767e02bf 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
>
> hdev = kzalloc(alloc_size, GFP_KERNEL);
> if (!hdev)
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
This will break all callers of hci_alloc_dev(). All callers expect NULL
in case of an error, so you will leave them with wrong pointer.
Also, allocation functionS return an error only in case of ENOMEM, so
initial code is fine, IMO
Thanks,
--Pavel Skripkin
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
2022-07-17 16:17 ` Pavel Skripkin
@ 2022-07-17 18:34 ` Khalid Masum
-1 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-17 18:34 UTC (permalink / raw)
To: Pavel Skripkin
Cc: linux-kernel-mentees, linux-kernel, Shuah Khan, Marcel Holtmann,
Johan Hedberg, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-bluetooth,
netdev
On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> Hi Khalid,
>
> Khalid Masum <khalid.masum.92@gmail.com> says:
> > Failure of kzalloc to allocate memory is not reported. Return Error
> > pointer to ENOMEM if memory allocation fails. This will increase
> > readability and will make the function easier to use in future.
> >
> > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > ---
>
> [snip]
>
> > index a0f99baafd35..ea50767e02bf 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> >
> > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > if (!hdev)
> > - return NULL;
> > + return ERR_PTR(-ENOMEM);
> >
>
> This will break all callers of hci_alloc_dev(). All callers expect NULL
> in case of an error, so you will leave them with wrong pointer.
You are right. All callers of hci_alloc_dev() need to be able to handle
the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
handling the ERR_PTR.
> Also, allocation functionS return an error only in case of ENOMEM, so
> initial code is fine, IMO
>
I think it makes the memory allocation error handling look to be a bit
different from what we usually do while allocating memory which is,
returning an error or an error pointer. Here we are returning a NULL
without any context, making it a bit unreadable. So I think returning
an error pointer is better. If I am not mistaken, this also complies with
the return convention:
https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
>
> Thanks,
> --Pavel Skripkin
Thanks,
-- Khalid Masum
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-17 18:34 ` Khalid Masum
0 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-17 18:34 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Johan Hedberg, netdev, Marcel Holtmann, linux-kernel,
linux-bluetooth, Eric Dumazet, Luiz Augusto von Dentz,
Jakub Kicinski, Paolo Abeni, linux-kernel-mentees,
David S . Miller
On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> Hi Khalid,
>
> Khalid Masum <khalid.masum.92@gmail.com> says:
> > Failure of kzalloc to allocate memory is not reported. Return Error
> > pointer to ENOMEM if memory allocation fails. This will increase
> > readability and will make the function easier to use in future.
> >
> > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > ---
>
> [snip]
>
> > index a0f99baafd35..ea50767e02bf 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> >
> > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > if (!hdev)
> > - return NULL;
> > + return ERR_PTR(-ENOMEM);
> >
>
> This will break all callers of hci_alloc_dev(). All callers expect NULL
> in case of an error, so you will leave them with wrong pointer.
You are right. All callers of hci_alloc_dev() need to be able to handle
the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
handling the ERR_PTR.
> Also, allocation functionS return an error only in case of ENOMEM, so
> initial code is fine, IMO
>
I think it makes the memory allocation error handling look to be a bit
different from what we usually do while allocating memory which is,
returning an error or an error pointer. Here we are returning a NULL
without any context, making it a bit unreadable. So I think returning
an error pointer is better. If I am not mistaken, this also complies with
the return convention:
https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
>
> Thanks,
> --Pavel Skripkin
Thanks,
-- Khalid Masum
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
2022-07-17 18:34 ` Khalid Masum
@ 2022-07-18 23:03 ` Luiz Augusto von Dentz
-1 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-18 23:03 UTC (permalink / raw)
To: Khalid Masum
Cc: Pavel Skripkin, linux-kernel-mentees, Linux Kernel Mailing List,
Shuah Khan, Marcel Holtmann, Johan Hedberg, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-bluetooth,
open list:NETWORKING [GENERAL]
Hi Khalid,
On Sun, Jul 17, 2022 at 11:34 AM Khalid Masum <khalid.masum.92@gmail.com> wrote:
>
> On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
> >
> > Hi Khalid,
> >
> > Khalid Masum <khalid.masum.92@gmail.com> says:
> > > Failure of kzalloc to allocate memory is not reported. Return Error
> > > pointer to ENOMEM if memory allocation fails. This will increase
> > > readability and will make the function easier to use in future.
> > >
> > > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > > ---
> >
> > [snip]
> >
> > > index a0f99baafd35..ea50767e02bf 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> > >
> > > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > > if (!hdev)
> > > - return NULL;
> > > + return ERR_PTR(-ENOMEM);
> > >
> >
> > This will break all callers of hci_alloc_dev(). All callers expect NULL
> > in case of an error, so you will leave them with wrong pointer.
>
> You are right. All callers of hci_alloc_dev() need to be able to handle
> the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
> handling the ERR_PTR.
>
> > Also, allocation functionS return an error only in case of ENOMEM, so
> > initial code is fine, IMO
> >
If there just a single error like ENOMEM then Id say this is fine,
just as it is fine for kzalloc.
> I think it makes the memory allocation error handling look to be a bit
> different from what we usually do while allocating memory which is,
> returning an error or an error pointer. Here we are returning a NULL
> without any context, making it a bit unreadable. So I think returning
> an error pointer is better. If I am not mistaken, this also complies with
> the return convention:
> https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
Not sure if that would apply to code that is basically a wrapper of kzalloc.
> >
> > Thanks,
> > --Pavel Skripkin
>
>
> Thanks,
> -- Khalid Masum
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-18 23:03 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-18 23:03 UTC (permalink / raw)
To: Khalid Masum
Cc: Johan Hedberg, Pavel Skripkin, Marcel Holtmann,
Linux Kernel Mailing List, linux-bluetooth, Eric Dumazet,
open list:NETWORKING [GENERAL],
Jakub Kicinski, Paolo Abeni, linux-kernel-mentees,
David S . Miller
Hi Khalid,
On Sun, Jul 17, 2022 at 11:34 AM Khalid Masum <khalid.masum.92@gmail.com> wrote:
>
> On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
> >
> > Hi Khalid,
> >
> > Khalid Masum <khalid.masum.92@gmail.com> says:
> > > Failure of kzalloc to allocate memory is not reported. Return Error
> > > pointer to ENOMEM if memory allocation fails. This will increase
> > > readability and will make the function easier to use in future.
> > >
> > > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > > ---
> >
> > [snip]
> >
> > > index a0f99baafd35..ea50767e02bf 100644
> > > --- a/net/bluetooth/hci_core.c
> > > +++ b/net/bluetooth/hci_core.c
> > > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> > >
> > > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > > if (!hdev)
> > > - return NULL;
> > > + return ERR_PTR(-ENOMEM);
> > >
> >
> > This will break all callers of hci_alloc_dev(). All callers expect NULL
> > in case of an error, so you will leave them with wrong pointer.
>
> You are right. All callers of hci_alloc_dev() need to be able to handle
> the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
> handling the ERR_PTR.
>
> > Also, allocation functionS return an error only in case of ENOMEM, so
> > initial code is fine, IMO
> >
If there just a single error like ENOMEM then Id say this is fine,
just as it is fine for kzalloc.
> I think it makes the memory allocation error handling look to be a bit
> different from what we usually do while allocating memory which is,
> returning an error or an error pointer. Here we are returning a NULL
> without any context, making it a bit unreadable. So I think returning
> an error pointer is better. If I am not mistaken, this also complies with
> the return convention:
> https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
Not sure if that would apply to code that is basically a wrapper of kzalloc.
> >
> > Thanks,
> > --Pavel Skripkin
>
>
> Thanks,
> -- Khalid Masum
--
Luiz Augusto von Dentz
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
2022-07-18 23:03 ` Luiz Augusto von Dentz
@ 2022-07-23 10:54 ` Khalid Masum
-1 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-23 10:54 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Pavel Skripkin, linux-kernel-mentees, Linux Kernel Mailing List,
Shuah Khan, Marcel Holtmann, Johan Hedberg, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-bluetooth,
open list:NETWORKING [GENERAL]
On Tue, Jul 19, 2022 at 5:04 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Khalid,
>
> On Sun, Jul 17, 2022 at 11:34 AM Khalid Masum <khalid.masum.92@gmail.com> wrote:
> >
> > On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
> > >
> > > Hi Khalid,
> > >
> > > Khalid Masum <khalid.masum.92@gmail.com> says:
> > > > Failure of kzalloc to allocate memory is not reported. Return Error
> > > > pointer to ENOMEM if memory allocation fails. This will increase
> > > > readability and will make the function easier to use in future.
> > > >
> > > > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > > > ---
> > >
> > > [snip]
> > >
> > > > index a0f99baafd35..ea50767e02bf 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> > > >
> > > > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > > > if (!hdev)
> > > > - return NULL;
> > > > + return ERR_PTR(-ENOMEM);
> > > >
> > >
> > > This will break all callers of hci_alloc_dev(). All callers expect NULL
> > > in case of an error, so you will leave them with wrong pointer.
> >
> > You are right. All callers of hci_alloc_dev() need to be able to handle
> > the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
> > handling the ERR_PTR.
> >
> > > Also, allocation functionS return an error only in case of ENOMEM, so
> > > initial code is fine, IMO
> > >
>
> If there just a single error like ENOMEM then Id say this is fine,
> just as it is fine for kzalloc.
>
> > I think it makes the memory allocation error handling look to be a bit
> > different from what we usually do while allocating memory which is,
> > returning an error or an error pointer. Here we are returning a NULL
> > without any context, making it a bit unreadable. So I think returning
> > an error pointer is better. If I am not mistaken, this also complies with
> > the return convention:
> > https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
>
> Not sure if that would apply to code that is basically a wrapper of kzalloc.
I got you.
> > > Thanks,
> > > --Pavel Skripkin
> >
> >
> > Thanks,
> > -- Khalid Masum
>
>
>
> --
> Luiz Augusto von Dentz
Thanks,
-- Khalid Masum
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL
@ 2022-07-23 10:54 ` Khalid Masum
0 siblings, 0 replies; 11+ messages in thread
From: Khalid Masum @ 2022-07-23 10:54 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Johan Hedberg, Pavel Skripkin, Marcel Holtmann,
Linux Kernel Mailing List, linux-bluetooth, Eric Dumazet,
open list:NETWORKING [GENERAL],
Jakub Kicinski, Paolo Abeni, linux-kernel-mentees,
David S . Miller
On Tue, Jul 19, 2022 at 5:04 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Khalid,
>
> On Sun, Jul 17, 2022 at 11:34 AM Khalid Masum <khalid.masum.92@gmail.com> wrote:
> >
> > On Sun, Jul 17, 2022 at 10:17 PM Pavel Skripkin <paskripkin@gmail.com> wrote:
> > >
> > > Hi Khalid,
> > >
> > > Khalid Masum <khalid.masum.92@gmail.com> says:
> > > > Failure of kzalloc to allocate memory is not reported. Return Error
> > > > pointer to ENOMEM if memory allocation fails. This will increase
> > > > readability and will make the function easier to use in future.
> > > >
> > > > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > > > ---
> > >
> > > [snip]
> > >
> > > > index a0f99baafd35..ea50767e02bf 100644
> > > > --- a/net/bluetooth/hci_core.c
> > > > +++ b/net/bluetooth/hci_core.c
> > > > @@ -2419,7 +2419,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
> > > >
> > > > hdev = kzalloc(alloc_size, GFP_KERNEL);
> > > > if (!hdev)
> > > > - return NULL;
> > > > + return ERR_PTR(-ENOMEM);
> > > >
> > >
> > > This will break all callers of hci_alloc_dev(). All callers expect NULL
> > > in case of an error, so you will leave them with wrong pointer.
> >
> > You are right. All callers of hci_alloc_dev() need to be able to handle
> > the error pointer. I shall send a V2 with all the callers of hci_alloc_dev
> > handling the ERR_PTR.
> >
> > > Also, allocation functionS return an error only in case of ENOMEM, so
> > > initial code is fine, IMO
> > >
>
> If there just a single error like ENOMEM then Id say this is fine,
> just as it is fine for kzalloc.
>
> > I think it makes the memory allocation error handling look to be a bit
> > different from what we usually do while allocating memory which is,
> > returning an error or an error pointer. Here we are returning a NULL
> > without any context, making it a bit unreadable. So I think returning
> > an error pointer is better. If I am not mistaken, this also complies with
> > the return convention:
> > https://www.kernel.org/doc/htmldocs/kernel-hacking/convention-returns.html
>
> Not sure if that would apply to code that is basically a wrapper of kzalloc.
I got you.
> > > Thanks,
> > > --Pavel Skripkin
> >
> >
> > Thanks,
> > -- Khalid Masum
>
>
>
> --
> Luiz Augusto von Dentz
Thanks,
-- Khalid Masum
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-07-23 10:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-17 13:37 [PATCH] Bluetooth: hci_core: Use ERR_PTR instead of NULL Khalid Masum
2022-07-17 13:37 ` Khalid Masum
2022-07-17 14:04 ` bluez.test.bot
2022-07-17 16:17 ` [PATCH] " Pavel Skripkin
2022-07-17 16:17 ` Pavel Skripkin
2022-07-17 18:34 ` Khalid Masum
2022-07-17 18:34 ` Khalid Masum
2022-07-18 23:03 ` Luiz Augusto von Dentz
2022-07-18 23:03 ` Luiz Augusto von Dentz
2022-07-23 10:54 ` Khalid Masum
2022-07-23 10:54 ` Khalid Masum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.