lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 19 Jan 2024 08:56:02 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Sam Protsenko <semen.protsenko@...aro.org>
Cc: krzysztof.kozlowski@...aro.org, alim.akhtar@...sung.com,
 gregkh@...uxfoundation.org, jirislaby@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
 andre.draszik@...aro.org, peter.griffin@...aro.org, kernel-team@...roid.com,
 willmcvicker@...gle.com
Subject: Re: [PATCH 17/18] tty: serial: samsung: shrink port feature flags to
 u8



On 1/16/24 19:03, Sam Protsenko wrote:
> On Wed, Jan 10, 2024 at 4:25 AM Tudor Ambarus <tudor.ambarus@...aro.org> wrote:
>>
>> There's a single flag defined as of now. Shrink the feature flags to u8
>> and aim for a better memory footprint for ``struct s3c24xx_uart_info``.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@...aro.org>
>> ---
>>  drivers/tty/serial/samsung_tty.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
>> index 5df2bcebf9fb..598d9fe7a492 100644
>> --- a/drivers/tty/serial/samsung_tty.c
>> +++ b/drivers/tty/serial/samsung_tty.c
>> @@ -90,7 +90,7 @@ struct s3c24xx_uart_info {
>>
>>         /* uart port features */
>>
>> -       unsigned int            has_divslot:1;
>> +       u8                      has_divslot:1;
> 
> But that's already a bit field. Why does it matter which type it is?
> 

If using unsigned int the bitfied is combined with the previous u8
fields, whereas if using u8 the bitfield will be independently defined.
So no benefit in terms of memory footprint, it's just a cosmetic change
to align the bitfield with the previous u8 fields. Allowing u32 for just
a bit can be misleading as one would ask itself where are the other
bits. Between a u32 bitfield and a bool a u8 bitfield seems like a good
compromise.

I'll update the commit message with this explanation in v2 because I'd
keep the patch, it makes the struct look cleaner. At the same time I
don't have a strong opinion, so if you'd like to see the patch dropped,
tell there, I'll be fine with it.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ