On Thu, 28 Mar 2024 at 15:38, Lukas Wunner <lukas@xxxxxxxxx> wrote: > > On Thu, Mar 28, 2024 at 11:13:07AM +0200, Ard Biesheuvel wrote: > > On Thu, 28 Mar 2024 at 10:21, Lukas Wunner <lukas@xxxxxxxxx> wrote: > > > On Tue, Mar 26, 2024 at 11:18:51AM +0100, Ard Biesheuvel wrote: > > > > Add a missing (u64) cast to alloc_min, which is passed into > > > > efi_random_alloc() as unsigned long, while efi_physical_addr_t is u64. > > > [...] > > > > --- a/drivers/firmware/efi/libstub/randomalloc.c > > > > +++ b/drivers/firmware/efi/libstub/randomalloc.c > > > > @@ -120,7 +120,7 @@ efi_status_t efi_random_alloc(unsigned long size, > > > > continue; > > > > } > > > > > > > > - target = round_up(max(md->phys_addr, alloc_min), align) + target_slot * align; > > > > + target = round_up(max(md->phys_addr, (u64)alloc_min), align) + target_slot * align; > > > > > > Why not > > > > > > max_t(u64, md->phys_addr, alloc_min) > > > > Why is that better? > > It just seems to be the idiomatic way to handle these casts in the kernel. > In this particular case, I prefer max() with the cast, because it matches the other occurrence, where alloc_min is also used but there it is u64 not unsigned long. > It's also what checkpatch suggests, so by not using it you risk getting > "helpful" fixup patches from the usual suspects. > Ugh yeah good point. > It's your call buddy. :) Thanks for the head's up
