On Fri, 13 Feb 2026 at 11:52, Javier Martinez Canillas <[email protected]> wrote: > > Ilias Apalodimas <[email protected]> writes: > > Hello Ilias, > > > Hi Javier > > > > [...] > > > >> unsigned char end_head; /* end head > >> */ > >> unsigned char end_sector; /* end sector > >> */ > >> unsigned char end_cyl; /* end cylinder > >> */ > >> - unsigned char start4[4]; /* starting sector counting from 0 > >> */ > >> - unsigned char size4[4]; /* nr of sectors in partition > >> */ > >> -} dos_partition_t; > >> + __le32 start_sect; /* starting sector counting from 0 > >> */ > >> + __le32 nr_sects; /* nr of sectors in partition > >> */ > >> +} __packed dos_partition_t; > > > > This always has to be packed and the part_dos.h definition had this > > wrong all along? > > > > I wondered exactly the same when working on these patches and did a bit > of code archaeology, so I'm glad that you asked! > > It wasn't wrong because it was using unsigned char arrays for the start > and number of sectors struct fields, instead of __le32 types. > > Given the alignment of a char array is 1-byte (and all the other fields > are unsigned chars), the structure was naturally aligned. > > But I agree that using __le32 and __packed is preferred, and in fact is > how Linux defines this data structure in include/linux/msdos_partition.h: > > struct msdos_partition { > u8 boot_ind; /* 0x80 - active */ > u8 head; /* starting head */ > u8 sector; /* starting sector */ > u8 cyl; /* starting cylinder */ > u8 sys_ind; /* What partition type */ > u8 end_head; /* end head */ > u8 end_sector; /* end sector */ > u8 end_cyl; /* end cylinder */ > __le32 start_sect; /* starting sector counting from 0 */ > __le32 nr_sects; /* nr of sectors in partition */ > } __packed; > > The struct_partition in disk/part_efi.c is said to be taken from Linux's > linux/include/genhd.h, which was the old location before commit 1442f76d4317b > ("1442f76d4317 block: move struct partition out of genhd.h"). > > My guess is that the old data structure defined in part_dos.h was copied from > Linux, and that at the time of the copy it was using unsigned char arrays too.
Ok that makes sense and yes, the alignment is not affected by le32 or a a 4b char. Reviewed-by: Ilias Apalodimas <[email protected]> > > But the struct definition has been like this since the original Linux git > import, so I couldn't check if my guess was correct. > > In any case, now this single data structure definition is aligned with how > both the Linux kernel and edk2 define it. > > -- > Best regards, > > Javier Martinez Canillas > Core Platforms > Red Hat >
