From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753376AbbB0Q0k (ORCPT ); Fri, 27 Feb 2015 11:26:40 -0500 Received: from mail-wg0-f47.google.com ([74.125.82.47]:32769 "EHLO mail-wg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752386AbbB0Q0h (ORCPT ); Fri, 27 Feb 2015 11:26:37 -0500 Message-ID: <54F09ABA.3080307@suse.cz> Date: Fri, 27 Feb 2015 17:26:34 +0100 From: Jiri Slaby User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Marcel Holtmann CC: Johan Hedberg , BlueZ development , Linux Kernel Mailing List , "Gustavo F. Padovan" Subject: Re: [PATCH v2 1/1] Bluetooth: make hci_test_bit's addr const References: <1424354164-11728-1-git-send-email-jslaby@suse.cz> <1424355643-30660-1-git-send-email-jslaby@suse.cz> <21570C6D-0C2F-40CA-9100-5A5CC7D1CEAB@holtmann.org> In-Reply-To: <21570C6D-0C2F-40CA-9100-5A5CC7D1CEAB@holtmann.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/19/2015, 05:10 PM, Marcel Holtmann wrote: > Hi Jiri, > >> gcc5 warns about passing a const array to hci_test_bit which takes a >> non-const pointer: >> net/bluetooth/hci_sock.c: In function ‘hci_sock_sendmsg’: >> net/bluetooth/hci_sock.c:955:8: warning: passing argument 2 of ‘hci_test_bit’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-array-qualifiers] >> &hci_sec_filter.ocf_mask[ogf])) && >> ^ >> net/bluetooth/hci_sock.c:49:19: note: expected ‘void *’ but argument is of type ‘const __u32 (*)[4] {aka const unsigned int (*)[4]}’ >> static inline int hci_test_bit(int nr, void *addr) >> ^ >> >> So make 'addr' 'const void *'. >> >> [v2] make the pointer in the cast const too. >> >> Signed-off-by: Jiri Slaby >> Cc: Marcel Holtmann >> Cc: Gustavo Padovan >> Cc: Johan Hedberg >> --- >> net/bluetooth/hci_sock.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c >> index 1d65c5be7c82..d1a7d67619d8 100644 >> --- a/net/bluetooth/hci_sock.c >> +++ b/net/bluetooth/hci_sock.c >> @@ -46,9 +46,9 @@ struct hci_pinfo { >> unsigned short channel; >> }; >> >> -static inline int hci_test_bit(int nr, void *addr) >> +static inline int hci_test_bit(int nr, const void *addr) >> { >> - return *((__u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31)); >> + return *((const __u32 *) addr + (nr >> 5)) & ((__u32) 1 << (nr & 31)); >> } > > we had a lengthy discussion about this before. I am reluctant to change any of this until we have a test tool that proofs this does not break userspace API compatibility. > > I don't see how just adding const would break anything, but I cautious since this code is pretty much a big mistake that we are carrying around for almost 14 years now. Yeah, I understand, but relying on the compiler is worse in this case, I think. Here, the function casts away const from a const array, which is undefined behaviour (6.7.3 of the standard). So all in all I believe applying this fix cannot make the code worse. thanks, -- js suse labs