ath11k: using boolean bitfields?

From: Kalle Valo <kvalo@codeaurora.org>
To: Sven Eckelmann <sven@narfation.org>
Cc: ath11k@lists.infradead.org, Wen Gong <quic_wgong@quicinc.com>,
	linux-wireless@vger.kernel.org, Jason Gunthorpe <jgg@ziepe.ca>,
	Joe Perches <joe@perches.com>, Jonathan Corbet <corbet@lwn.net>
Subject: ath11k: using boolean bitfields?
Date: Fri, 19 Nov 2021 17:17:08 +0200	[thread overview]
Message-ID: <87v90ob2ff.fsf_-_@codeaurora.org> (raw)
In-Reply-To: <2357945.8AGfkZV0UB@sven-desktop> (Sven Eckelmann's message of "Wed, 17 Nov 2021 09:46:08 +0100")

Sven Eckelmann <sven@narfation.org> writes:

> On Wednesday, 17 November 2021 09:12:55 CET Kalle Valo wrote:
>> > https://www.kernel.org/doc/html/v5.15/process/coding-style.html#using-bool
> [...]
>> 
>> Yeah, I have been worried about this as well and we should fix this. But
>> instead of u8 I would prefer to use bool like mt76 uses:
> [...]
>> I didn't even know using bool is legal until I saw it in mt76.
>
> Interesting, I was also not aware of it. And it also seems to have some 
> interesting implications when assigning values to it (example 4):
>
>     #include <stdbool.h>
>     #include <stdint.h>
>     #include <stdio.h>
>     
>     struct test {
>     	uint8_t u:1;
>     	uint8_t u2:1;
>     	bool b:1;
>     	bool b2:1;
>     };
>     
>     int main(void)
>     {
>     	struct test x;
>     
>     	x.u = false;
>     	x.b = false;
>     	printf("u %u b %u\n", x.u, x.b);
>     
>     	x.u = true;
>     	x.b = true;
>     	printf("u %u b %u\n", x.u, x.b);
>     
>     	x.u = 0;
>     	x.b = 0;
>     	printf("u %u b %u\n", x.u, x.b);
>     
>     	x.u = 8;
>     	x.b = 8;
>     	printf("u %u b %u\n", x.u, x.b);
>     
>     	return 0;
>     }
>
>
> Result:
>
>     u 0 b 0
>     u 1 b 1
>     u 0 b 0
>     u 0 b 1
>
>
> The last example is basically the reason we see stuff like
>
>     boolean_like_value = !!(some_retrieved_value);
>
> when using unsigned bitfields instead of bool (bitfields).
>
>
> And the memory layout (on x86-64):
>
>     $ pahole test.o                                    
>     struct test {
>             uint8_t                    u:1;                  /*     0: 0  1 */
>             uint8_t                    u2:1;                 /*     0: 1  1 */
>             _Bool                      b:1;                  /*     0: 2  1 */
>             _Bool                      b2:1;                 /*     0: 3  1 */
>     
>             /* size: 1, cachelines: 1, members: 4 */
>             /* bit_padding: 4 bits */
>             /* last cacheline: 1 bytes */
>     };
>
>
> To my surprise, it was already mentioned in one of the discussions [1].
> Was there anything in the discussion which I might have missed and 
> is a good reason to not use "bool ...:1" in structs?

No responses so must be safe to use ;) Changing the subject to gain more
visibility. But IMHO we should just switch using boolean bitfields as I
would expect mt76 to have noticed any problems by now.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k