From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org,
Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
linux-bluetooth@vger.kernel.org
Cc: lkp@intel.com, kbuild-all@lists.01.org
Subject: Re: [PATCH v2 5/8] Bluetooth: Add initial implementation of BIS connections
Date: Thu, 12 May 2022 13:51:07 +0300 [thread overview]
Message-ID: <202205121532.jtQf7nFA-lkp@intel.com> (raw)
In-Reply-To: <20220506215743.3870212-5-luiz.dentz@gmail.com>
Hi Luiz,
url: https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-eir-Add-helpers-for-managing-service-data/20220507-060014
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: csky-randconfig-m031-20220508 (https://download.01.org/0day-ci/archive/20220512/202205121532.jtQf7nFA-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 11.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
net/bluetooth/hci_sync.c:1110 hci_start_per_adv_sync() warn: passing zero to 'PTR_ERR'
vim +/PTR_ERR +1110 net/bluetooth/hci_sync.c
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1091 int hci_start_per_adv_sync(struct hci_dev *hdev, u8 instance, u8 data_len,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1092 u8 *data, u32 flags, u16 min_interval,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1093 u16 max_interval, u16 sync_interval)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1094 {
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1095 struct adv_info *adv = NULL;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1096 int err;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1097 bool added = false;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1098
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1099 hci_disable_per_advertising_sync(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1100
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1101 if (instance) {
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1102 adv = hci_find_adv_instance(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1103 /* Create an instance if that could not be found */
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1104 if (!adv) {
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1105 adv = hci_add_per_instance(hdev, instance, flags,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1106 data_len, data,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1107 sync_interval,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1108 sync_interval);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1109 if (IS_ERR_OR_NULL(adv))
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 @1110 return PTR_ERR(adv);
If hci_add_per_instance() returns NULL then do we really want to return
success here?
When functions return both error pointers and NULL, the NULL means the
feature has been disabled deliberately. So, for example, some people
like blinking LED lights but others do not want them.
p = get_blinking_lights();
If there is an allocation failure or some kind of an error then p is
an error pointer. We have to return that error to the user. It might
seem like a small thing to us, but maybe it's important to them for some
reason so we can't just ignore their desire for blinking lights. We
have to handle that error properly so they can fix the issue and retry.
However if p is NULL then don't print a warning message, and don't
return an error. All the surrounding code has to be written to support
that blinking lights are optional.
The hci_add_per_instance() function is not optional and it cannot return
NULL.
There are some examples where this is done incorrectly. Sometimes
people think checking for NULL is just safer because they assume people
will add bugs to hci_add_per_instance() so that it returns the wrong
thing. This occasionally does happen. I fixed a bug earlier this week.
But we generally don't work around bugs, we just fix them.
There is another code which returns ERR_PTR(-EINVAL) when the feature is
disabled. It also returns -EINVAL for other errors. The callers have
to do something like:
p = get_feature();
if (p == ERR_PTR(-EINVAL))
p = NULL;
if (IS_ERR(p))
return PTR_ERR(p);
So not everything conforms to the guidelines.
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1111 added = true;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1112 }
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1113 }
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1114
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1115 /* Only start advertising if instance 0 or if a dedicated instance has
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1116 * been added.
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1117 */
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1118 if (!adv || added) {
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1119 err = hci_start_ext_adv_sync(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1120 if (err < 0)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1121 goto fail;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1122
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1123 err = hci_adv_bcast_annoucement(hdev, adv);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1124 if (err < 0)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1125 goto fail;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1126 }
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1127
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1128 err = hci_set_per_adv_params_sync(hdev, instance, min_interval,
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1129 max_interval);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1130 if (err < 0)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1131 goto fail;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1132
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1133 err = hci_set_per_adv_data_sync(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1134 if (err < 0)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1135 goto fail;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1136
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1137 err = hci_enable_per_advertising_sync(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1138 if (err < 0)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1139 goto fail;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1140
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1141 return 0;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1142
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1143 fail:
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1144 if (added)
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1145 hci_remove_adv_instance(hdev, instance);
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1146
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1147 return err;
2f80269de71810 Luiz Augusto von Dentz 2022-05-06 1148 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp
next prev parent reply other threads:[~2022-05-12 10:51 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-06 21:57 [PATCH v2 1/8] Bluetooth: eir: Add helpers for managing service data Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 2/8] Bluetooth: hci_core: Introduce hci_recv_event_data Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 3/8] Bluetooth: Add initial implementation of CIS connections Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 4/8] Bluetooth: Add BTPROTO_ISO socket type Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 5/8] Bluetooth: Add initial implementation of BIS connections Luiz Augusto von Dentz
2022-05-12 10:51 ` Dan Carpenter [this message]
2022-05-06 21:57 ` [PATCH v2 6/8] Bluetooth: ISO: Add broadcast support Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 7/8] Bluetooth: btusb: Add support for ISO packets Luiz Augusto von Dentz
2022-05-06 21:57 ` [PATCH v2 8/8] Bluetooth: btusb: Detect if an ACL packet is in fact an ISO packet Luiz Augusto von Dentz
2022-05-06 23:04 ` [v2,1/8] Bluetooth: eir: Add helpers for managing service data bluez.test.bot
2022-05-19 18:20 ` [PATCH v2 1/8] " patchwork-bot+bluetooth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202205121532.jtQf7nFA-lkp@intel.com \
--to=dan.carpenter@oracle.com \
--cc=kbuild-all@lists.01.org \
--cc=kbuild@lists.01.org \
--cc=linux-bluetooth@vger.kernel.org \
--cc=lkp@intel.com \
--cc=luiz.dentz@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).