Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 12 May 2022 13:51:07 +0300
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
Message-ID: <202205121532.jtQf7nFA-lkp@intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220506215743.3870212-5-luiz.dentz@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?N7qAedWxGxPHQ1ZszkiNCoWak0dkvdCpRGlay5BnGjuxIKYuKDVTwnaP2JJ3?=
 =?us-ascii?Q?MF9kmDtcvhuTZItFTls4aw6mpKCDzoYWankbbrrdupU3y2yHb8CtXk+8IWcF?=
 =?us-ascii?Q?v6yefD7JHORJ15D0jjO6ioZ3ivCsOs2KYe5NyAFd+RmeDfKdLH1zbszjpg3c?=
 =?us-ascii?Q?iDF0SXXxQtxaTPVZght9aV/KjDJUdVAaqqANJPpAB7xa/rFzWsUz/LW65RPX?=
 =?us-ascii?Q?oh+liItPTLSge8aa/4l8R2Bg5V5L97UsyXY+3dHpSIYbgVESEkTPgbYfPJBn?=
 =?us-ascii?Q?DvHDaezkJMuVsJRR95900l8MMliUnz53DL08dajpcbDXLZPcl+iUQcyKYvql?=
 =?us-ascii?Q?S6QxHhsTu9xjcJjAdWyn/+i8aArkUHe/50bz19RYfrk4i8kFFHJrqFVC3IQa?=
 =?us-ascii?Q?2Y9cnneyIPI0RQmRLCycfDPIElCXiucjbmPvWWMStAnxmGHYk5MudvESGTbr?=
 =?us-ascii?Q?1WPUSFwGtrnp61VT9gkez12YREXSyg2octf8ZR76TPHinpKvoGoqU0YTeg3u?=
 =?us-ascii?Q?QrSA+jHcktGHGWz1dgPTlDo4E1gdBIDFhh0Q+hY6TNbtVzp1DB1livRhgcFg?=
 =?us-ascii?Q?yVFo9CeNkoU4kGqZBS6k2eKsVESyEXJ3g2DDWDJaP2hEOXhVpd7PijLpQpph?=
 =?us-ascii?Q?2SOTe89kwTestGDZUW7eP3FedcUoKyA49RS8mQtaEFAJTi6i+gnatdTLM+nl?=
 =?us-ascii?Q?2a6Ld4LgH4gPtAtqIgDG9gVug/AsHmE2wq2RGxj9ubMZ961bp3/6ygf/C8vC?=
 =?us-ascii?Q?o5Gj7ZOvIII4ndzD1RTnDKTLxcOyYCXdaUaNd5MZevh0C1gjPoV6QmqRsTHm?=
 =?us-ascii?Q?FJ8MbvJ4y3vOupnCYvNZYGG8Ef/4E36Vc/j4SU1+Q3blddx0g9w7iWMMlfuy?=
 =?us-ascii?Q?mZWINvrI40zn+3+uCyxp2k5YgzUPi0lnx6iDJMjwp92yrOkDGbaz6IG/4QAl?=
 =?us-ascii?Q?KrJBmNHsGBMEz9CMzbj9WOO4SxAiJZndp2YR/lmaAa2mWld5Yj2D5YntPxhs?=
 =?us-ascii?Q?jwHRIc5M6ylCryxRn9FaGk6MUCZI9rTXxVt1dMNEjjtihGj2L5/UDyruj7fq?=
 =?us-ascii?Q?x7mbvYDoK/aGnWggUCybn/j+z3TWP+HTeLKtq1X9eYVITcSPeW9OuBHiBnTL?=
 =?us-ascii?Q?m6oUmAL1ihHMD2S+2Xrq9a2mV8NbHpu1q9fNz1tXOZawQ1aTtn5dgT4q1KxW?=
 =?us-ascii?Q?5ZtdEUpKaxrnbudN+vpcqFhUwhjcB+d05ZilGRDy0yotUMr444CcucE+vvRs?=
 =?us-ascii?Q?hodNtTbU/Ui5oGw5jAnrpx4/9xi+9Ut8wJnlWW2pqv9S9jQo89kHhXk+5owr?=
 =?us-ascii?Q?2mH2M0SVSkCwmBMku8qtAzy/F6S15stEMGGaJcVlwIH5IQu/g6R5Haq0G9Yz?=
 =?us-ascii?Q?Bqi5SScsW/gZdYv5XKyCD//DHHIp4U3jcOkwDULaZKJyhTO7NSYraMe/L3qF?=
 =?us-ascii?Q?4BHqpzhaU04GVPgw9Djz6rvgDDvZiGW++K2nvGtUynS1IL7j7WUCAG3ODqz6?=
 =?us-ascii?Q?cKw+DEUEPQp9ROXT3I5DoBr0+WbZ1a0h3zczTIBqZxqShI89uEm7mteJ1E5R?=
 =?us-ascii?Q?UA0KQ693S6apj/+vI5KI24N52EE4SS8msP6NmVsp0MCbLmZ4jI13UVpbP4X6?=
 =?us-ascii?Q?5MpvIO6B0bgHK8ac3EL8+iOcD1NLz+umUvmIwPg3OX4Yjvc1P7oA8F8Gjhye?=
 =?us-ascii?Q?SrTWZrzuPEp8vEUEIIrwLIcLRyNnHC/ksJoCTYD9LbYQGkPiQwgpB7mEOhLs?=
 =?us-ascii?Q?oU+TcxBHDqYQaqJSC8i0fFHqHtAqX2A=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ef15fc12-11ae-47f1-a3fb-08da340557ef
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2022 10:51:18.7243
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SeQ8lzs7NbFnvweFn1MR5psf3fPr5rkuLpcShlym9dLF/N7ZQanXKjvXn49MWkv2VzB8PC9+WJkO3qokbjvOAdlk/Kiz8MSxO7zm1FQWGSM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6274
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-12_02:2022-05-12,2022-05-12 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 mlxscore=0 phishscore=0 spamscore=0 bulkscore=0 adultscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205120049
X-Proofpoint-GUID: leEKmEVe79MSBnKIK8WibTZ5EFId7PyH
X-Proofpoint-ORIG-GUID: leEKmEVe79MSBnKIK8WibTZ5EFId7PyH
X-Spam-Status: No, score=-1.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FAKE_REPLY_C,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

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