[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-09 Thread Harshit Sharma
Thanks for your explanation. I do understand now how '1 << 31' could cause a potential problem. On Fri, Jan 8, 2021 at 6:10 AM Nico Huber wrote: > Hi Harshit, > > On 08.01.21 01:35, Harshit Sharma wrote: > > Although, I agree that 1u << 31 is much better, I believe 1 << 31 is not > > wrong

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-08 Thread Nico Huber
Hi Harshit, On 08.01.21 01:35, Harshit Sharma wrote: > Although, I agree that 1u << 31 is much better, I believe 1 << 31 is not > wrong either as long as it is assigned to an 'unsigned int' as the compiler > performs an implicit conversion from a lower data type to a higher data > type ('int' to

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-08 Thread Nico Huber
Hi, On 08.01.21 01:26, Peter Stuge wrote: > Nico Huber wrote: >> So, it's wrong but not broken > > ..yet that's right. But I think chances are incredibly low that C compilers would change this behavior. Of all the things that I'm worried about that could change, this case -- shifting out of the

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-08 Thread Nico Huber
Hi Peter, On 08.01.21 01:12, Peter Stuge wrote: > Felix Held wrote: >> While I find the BIT() macro to be much better than the BITx defines > > Why? for me it's that the BITx defines provide no separation between the BIT and the number. Also, the all-caps BIT letters come close to decimal

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Julius Werner
I'm someone who really doesn't like it. The U in 1U << 31 is unnecessary visual clutter that really makes the number harder to read (I guess 1u is slightly better but not by that much). I think we should define our code style first and foremost to make code easily written and easily read (with no

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Angel Pons
Hi list, Since register fields usually have more than one bit, I prefer to always use explicit shifts for consistency. The one case where I prefer the BIT() macro is to reduce the amount of nested braces when testing for individual bits in a mask. GCC encourages adding unnecessary braces for

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Harshit Sharma
Hi, Although, I agree that 1u << 31 is much better, I believe 1 << 31 is not wrong either as long as it is assigned to an 'unsigned int' as the compiler performs an implicit conversion from a lower data type to a higher data type ('int' to 'unsigned int' in this case). That's the reason the line

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Felix Held
While I find the BIT() macro to be much better than the BITx defines Why? The BITx defines seem to be a rather redundant way of doing things to me. I don't think it was invented by edk2, so edk2 using it shouldn't be held against the format. :) Sure, but that's where I remember seeing

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Peter Stuge
Nico Huber wrote: > So, it's wrong but not broken ..yet > What do you think? Should we allow such changes? I think yes. > Should we normalize on any style? There is an argument to be made for allowing many different styles, but I think it would be wise to standardize. > my personal

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Peter Stuge
Felix Held wrote: > While I find the BIT() macro to be much better than the BITx defines Why? I don't think it was invented by edk2, so edk2 using it shouldn't be held against the format. :) > header files become a mix of BIT() and more than one bit shifted by x > bits, which i find

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Felix Held
Hi, While I find the BIT() macro to be much better than the BITx defines in edk2, I still prefer the non-macro form and at least in the subtrees I maintain I try to get rid of BIT() usage in new code that gets merged. Since BIT() can only be used for single bits and not for multiple bits and

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Tim Wawrzynczak via coreboot
I will admit that I am a big fan of the `BIT` macro as well, but that seems to not be as well-received, so if that's not an option, I would second `1u << 31` On Thu, Jan 7, 2021 at 3:25 PM Furquan Shaikh wrote: > Would it make sense to use BIT(31)? > > On Thu, Jan 7, 2021 at 2:20 PM Matt

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Furquan Shaikh
Would it make sense to use BIT(31)? On Thu, Jan 7, 2021 at 2:20 PM Matt DeVillier wrote: > > `1u << 31` is correct, clear, and easy to read/understand quickly. It > has my vote. > > On Thu, Jan 7, 2021 at 3:42 PM Nico Huber wrote: > > > > Hi coreboot fellows, > > > > another patch that fixes a

[coreboot] Re: "Fixing" `1 << 31` (technically undefined behavior with known implementation-specific results)

2021-01-07 Thread Matt DeVillier
`1u << 31` is correct, clear, and easy to read/understand quickly. It has my vote. On Thu, Jan 7, 2021 at 3:42 PM Nico Huber wrote: > > Hi coreboot fellows, > > another patch that fixes a `1 << 31` to `1UL << 31` [1] reminded me that > some people objected to such changes. I'm not sure if we