Re: [RFC PATCH 2/6] block: partitions: efi: Fix some style issues

From: Davidlohr Bueso
Date: Mon Dec 11 2023 - 19:58:21 EST


On Mon, 11 Dec 2023, Romain Gantois wrote:

The block layer EFI code is quite old and does not perfectly match the
current kernel coding style. Fix some indentation and trailing whitespace
issues in efi.c.

I agree that the styling can use some love. However, a few comments below
where such changes are really unnecessary.

...

@@ -213,7 +212,7 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
*/
if (ret == GPT_MBR_PROTECTIVE) {
sz = le32_to_cpu(mbr->partition_record[part].size_in_lba);
- if (sz != (uint32_t) total_sectors - 1 && sz != 0xFFFFFFFF)
+ if (sz != (uint32_t)total_sectors - 1 && sz != 0xFFFFFFFF)

Like here.

pr_debug("GPT: mbr size in lba (%u) different than whole disk (%u).\n",
sz, min_t(uint32_t,
total_sectors - 1, 0xFFFFFFFF));
@@ -235,17 +234,19 @@ static int is_pmbr_valid(legacy_mbr *mbr, sector_t total_sectors)
static size_t read_lba(struct parsed_partitions *state,
u64 lba, u8 *buffer, size_t count)
{
- size_t totalreadcount = 0;
sector_t n = lba *
(queue_logical_block_size(state->disk->queue) / 512);
+ size_t totalreadcount = 0;
+ unsigned char *data;
+ Sector sect;
+ int copied;

if (!buffer || lba > last_lba(state->disk))
- return 0;
+ return 0;

while (count) {
- int copied = 512;
- Sector sect;
- unsigned char *data = read_part_sector(state, n++, &sect);
+ copied = 512;
+ data = read_part_sector(state, n++, &sect);

ditto

if (!data)
break;
if (copied > count)
@@ -253,7 +254,7 @@ static size_t read_lba(struct parsed_partitions *state,
memcpy(buffer, data, copied);
put_dev_sector(sect);
buffer += copied;
- totalreadcount +=copied;
+ totalreadcount += copied;
count -= copied;
}
return totalreadcount;
@@ -263,7 +264,7 @@ static size_t read_lba(struct parsed_partitions *state,
* alloc_read_gpt_entries(): reads partition entries from disk
* @state: disk parsed partitions
* @gpt: GPT header
- *
+ *
* Description: Returns ptes on success, NULL on error.
* Allocates space for PTEs based on information found in @gpt.
* Notes: remember to free pte when you're done!
@@ -271,14 +272,14 @@ static size_t read_lba(struct parsed_partitions *state,
static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
gpt_header *gpt)
{
- size_t count;
gpt_entry *pte;
+ size_t count;

ditto


if (!gpt)
return NULL;

count = (size_t)le32_to_cpu(gpt->num_partition_entries) *
- le32_to_cpu(gpt->sizeof_partition_entry);
+ le32_to_cpu(gpt->sizeof_partition_entry);
if (!count)
return NULL;
pte = kmalloc(count, GFP_KERNEL);
@@ -286,9 +287,9 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
return NULL;

if (read_lba(state, le64_to_cpu(gpt->partition_entry_lba),
- (u8 *) pte, count) < count) {
+ (u8 *)pte, count) < count) {
kfree(pte);
- pte=NULL;
+ pte = NULL;
return NULL;
}
return pte;
@@ -298,7 +299,7 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
* alloc_read_gpt_header(): Allocates GPT header, reads into it from disk
* @state: disk parsed partitions
* @lba: the Logical Block Address of the partition table
- *
+ *
* Description: returns GPT header on success, NULL on error. Allocates
* and fills a GPT header starting at @ from @state->disk.
* Note: remember to free gpt when finished with it.
@@ -306,16 +307,16 @@ static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state,
static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state,
u64 lba)
{
+ unsigned int ssz = queue_logical_block_size(state->disk->queue);
gpt_header *gpt;
- unsigned ssz = queue_logical_block_size(state->disk->queue);

ditto


gpt = kmalloc(ssz, GFP_KERNEL);
if (!gpt)
return NULL;

- if (read_lba(state, lba, (u8 *) gpt, ssz) < ssz) {
+ if (read_lba(state, lba, (u8 *)gpt, ssz) < ssz) {

ditto

kfree(gpt);
- gpt=NULL;
+ gpt = NULL;
return NULL;
}

@@ -486,31 +487,31 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
if (le64_to_cpu(pgpt->my_lba) != le64_to_cpu(agpt->alternate_lba)) {
pr_warn("GPT:Primary header LBA != Alt. header alternate_lba\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->my_lba),
- (unsigned long long)le64_to_cpu(agpt->alternate_lba));
+ (unsigned long long)le64_to_cpu(pgpt->my_lba),
+ (unsigned long long)le64_to_cpu(agpt->alternate_lba));
error_found++;
}
if (le64_to_cpu(pgpt->alternate_lba) != le64_to_cpu(agpt->my_lba)) {
pr_warn("GPT:Primary header alternate_lba != Alt. header my_lba\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
- (unsigned long long)le64_to_cpu(agpt->my_lba));
+ (unsigned long long)le64_to_cpu(pgpt->alternate_lba),
+ (unsigned long long)le64_to_cpu(agpt->my_lba));
error_found++;
}
if (le64_to_cpu(pgpt->first_usable_lba) !=
- le64_to_cpu(agpt->first_usable_lba)) {
+ le64_to_cpu(agpt->first_usable_lba)) {
pr_warn("GPT:first_usable_lbas don't match.\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
+ (unsigned long long)le64_to_cpu(pgpt->first_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->first_usable_lba));
error_found++;
}
if (le64_to_cpu(pgpt->last_usable_lba) !=
- le64_to_cpu(agpt->last_usable_lba)) {
+ le64_to_cpu(agpt->last_usable_lba)) {
pr_warn("GPT:last_usable_lbas don't match.\n");
pr_warn("GPT:%lld != %lld\n",
- (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
- (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
+ (unsigned long long)le64_to_cpu(pgpt->last_usable_lba),
+ (unsigned long long)le64_to_cpu(agpt->last_usable_lba));
error_found++;
}
if (efi_guidcmp(pgpt->disk_guid, agpt->disk_guid)) {
@@ -518,27 +519,24 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
error_found++;
}
if (le32_to_cpu(pgpt->num_partition_entries) !=
- le32_to_cpu(agpt->num_partition_entries)) {
- pr_warn("GPT:num_partition_entries don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->num_partition_entries),
- le32_to_cpu(agpt->num_partition_entries));
+ le32_to_cpu(agpt->num_partition_entries)) {
+ pr_warn("GPT:num_partition_entries don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->num_partition_entries),
+ le32_to_cpu(agpt->num_partition_entries));
error_found++;
}
if (le32_to_cpu(pgpt->sizeof_partition_entry) !=
- le32_to_cpu(agpt->sizeof_partition_entry)) {
- pr_warn("GPT:sizeof_partition_entry values don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->sizeof_partition_entry),
- le32_to_cpu(agpt->sizeof_partition_entry));
+ le32_to_cpu(agpt->sizeof_partition_entry)) {
+ pr_warn("GPT:sizeof_partition_entry values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->sizeof_partition_entry),
+ le32_to_cpu(agpt->sizeof_partition_entry));
error_found++;
}
if (le32_to_cpu(pgpt->partition_entry_array_crc32) !=
- le32_to_cpu(agpt->partition_entry_array_crc32)) {
- pr_warn("GPT:partition_entry_array_crc32 values don't match: "
- "0x%x != 0x%x\n",
- le32_to_cpu(pgpt->partition_entry_array_crc32),
- le32_to_cpu(agpt->partition_entry_array_crc32));
+ le32_to_cpu(agpt->partition_entry_array_crc32)) {
+ pr_warn("GPT:partition_entry_array_crc32 values don't match: 0x%x != 0x%x\n",
+ le32_to_cpu(pgpt->partition_entry_array_crc32),
+ le32_to_cpu(agpt->partition_entry_array_crc32));
error_found++;
}
if (le64_to_cpu(pgpt->alternate_lba) != lastlba) {
@@ -581,20 +579,22 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba)
static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
gpt_entry **ptes)
{
+ sector_t total_sectors = get_capacity(state->disk);
int good_pgpt = 0, good_agpt = 0, good_pmbr = 0;
- gpt_header *pgpt = NULL, *agpt = NULL;
+ const struct block_device_operations *fops;
gpt_entry *pptes = NULL, *aptes = NULL;
- legacy_mbr *legacymbr;
+ gpt_header *pgpt = NULL, *agpt = NULL;
struct gendisk *disk = state->disk;
- const struct block_device_operations *fops = disk->fops;
- sector_t total_sectors = get_capacity(state->disk);
+ legacy_mbr *legacymbr;
u64 lastlba;

+ fops = disk->fops;

ditto

+
if (!ptes)
return 0;

lastlba = last_lba(state->disk);
- if (!force_gpt) {
+ if (!force_gpt) {
/* This will be added to the EFI Spec. per Intel after v1.02. */
legacymbr = kzalloc(sizeof(*legacymbr), GFP_KERNEL);
if (!legacymbr)
@@ -609,17 +609,17 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,

pr_debug("Device has a %s MBR\n",
good_pmbr == GPT_MBR_PROTECTIVE ?
- "protective" : "hybrid");
+ "protective" : "hybrid");
}

good_pgpt = is_gpt_valid(state, GPT_PRIMARY_PARTITION_TABLE_LBA,
&pgpt, &pptes);
- if (good_pgpt)
+ if (good_pgpt)
good_agpt = is_gpt_valid(state,
le64_to_cpu(pgpt->alternate_lba),
&agpt, &aptes);
- if (!good_agpt && force_gpt)
- good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);
+ if (!good_agpt && force_gpt)
+ good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes);

if (!good_agpt && force_gpt && fops->alternative_gpt_sector) {
sector_t agpt_sector;
@@ -631,39 +631,38 @@ static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt,
&agpt, &aptes);
}

- /* The obviously unsuccessful case */
- if (!good_pgpt && !good_agpt)
- goto fail;
+ /* The obviously unsuccessful case */
+ if (!good_pgpt && !good_agpt)
+ goto fail;

compare_gpts(pgpt, agpt, lastlba);

- /* The good cases */
- if (good_pgpt) {
- *gpt = pgpt;
- *ptes = pptes;
- kfree(agpt);
- kfree(aptes);
+ /* The good cases */
+ if (good_pgpt) {
+ *gpt = pgpt;
+ *ptes = pptes;
+ kfree(agpt);
+ kfree(aptes);
if (!good_agpt)
- pr_warn("Alternate GPT is invalid, using primary GPT.\n");
- return 1;
- }
- else if (good_agpt) {
- *gpt = agpt;
- *ptes = aptes;
- kfree(pgpt);
- kfree(pptes);
+ pr_warn("Alternate GPT is invalid, using primary GPT.\n");
+ return 1;
+ } else if (good_agpt) {
+ *gpt = agpt;
+ *ptes = aptes;
+ kfree(pgpt);
+ kfree(pptes);
pr_warn("Primary GPT is invalid, using alternate GPT.\n");
- return 1;
- }
+ return 1;
+ }

- fail:
- kfree(pgpt);
- kfree(agpt);
- kfree(pptes);
- kfree(aptes);
- *gpt = NULL;
- *ptes = NULL;
- return 0;
+fail:
+ kfree(pgpt);
+ kfree(agpt);
+ kfree(pptes);
+ kfree(aptes);
+ *gpt = NULL;
+ *ptes = NULL;
+ return 0;
}

/**
@@ -712,10 +711,10 @@ static void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
*/
int efi_partition(struct parsed_partitions *state)
{
+ unsigned int ssz = queue_logical_block_size(state->disk->queue) / 512;
gpt_header *gpt = NULL;
gpt_entry *ptes = NULL;
u32 i;
- unsigned ssz = queue_logical_block_size(state->disk->queue) / 512;

ditto


if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
kfree(gpt);
@@ -725,17 +724,17 @@ int efi_partition(struct parsed_partitions *state)

pr_debug("GUID Partition Table is valid! Yea!\n");

- for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit-1; i++) {
+ for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) && i < state->limit - 1; i++) {

ditto

struct partition_meta_info *info;
- unsigned label_max;
+ unsigned int label_max;
u64 start = le64_to_cpu(ptes[i].starting_lba);
u64 size = le64_to_cpu(ptes[i].ending_lba) -
- le64_to_cpu(ptes[i].starting_lba) + 1ULL;
+ le64_to_cpu(ptes[i].starting_lba) + 1ULL;

if (!is_pte_valid(&ptes[i], last_lba(state->disk)))
continue;

- put_partition(state, i+1, start * ssz, size * ssz);
+ put_partition(state, i + 1, start * ssz, size * ssz);

/* If this is a RAID volume, tell md */
if (!efi_guidcmp(ptes[i].partition_type_guid, PARTITION_LINUX_RAID_GUID))
diff --git a/include/linux/gpt.h b/include/linux/gpt.h
index 84b9f36b9e47..633be6bc826c 100644
--- a/include/linux/gpt.h
+++ b/include/linux/gpt.h
@@ -4,7 +4,7 @@
* Per Intel EFI Specification v1.02
* http://developer.intel.com/technology/efi/efi.htm
*
- * By Matt Domsch <Matt_Domsch@xxxxxxxx> Fri Sep 22 22:15:56 CDT 2000
+ * By Matt Domsch <Matt_Domsch@xxxxxxxx> Fri Sep 22 22:15:56 CDT 2000
* Copyright 2000,2001 Dell Inc.
************************************************************/

@@ -31,26 +31,26 @@
#define GPT_PRIMARY_PARTITION_TABLE_LBA 1

#define PARTITION_SYSTEM_GUID \
- EFI_GUID( 0xC12A7328, 0xF81F, 0x11d2, \
- 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
+ EFI_GUID(0xC12A7328, 0xF81F, 0x11d2, \
+ 0xBA, 0x4B, 0x00, 0xA0, 0xC9, 0x3E, 0xC9, 0x3B)
#define LEGACY_MBR_PARTITION_GUID \
- EFI_GUID( 0x024DEE41, 0x33E7, 0x11d3, \
- 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
+ EFI_GUID(0x024DEE41, 0x33E7, 0x11d3, \
+ 0x9D, 0x69, 0x00, 0x08, 0xC7, 0x81, 0xF3, 0x9F)
#define PARTITION_MSFT_RESERVED_GUID \
- EFI_GUID( 0xE3C9E316, 0x0B5C, 0x4DB8, \
- 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
+ EFI_GUID(0xE3C9E316, 0x0B5C, 0x4DB8, \
+ 0x81, 0x7D, 0xF9, 0x2D, 0xF0, 0x02, 0x15, 0xAE)
#define PARTITION_BASIC_DATA_GUID \
- EFI_GUID( 0xEBD0A0A2, 0xB9E5, 0x4433, \
- 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
+ EFI_GUID(0xEBD0A0A2, 0xB9E5, 0x4433, \
+ 0x87, 0xC0, 0x68, 0xB6, 0xB7, 0x26, 0x99, 0xC7)
#define PARTITION_LINUX_RAID_GUID \
- EFI_GUID( 0xa19d880f, 0x05fc, 0x4d3b, \
- 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
+ EFI_GUID(0xa19d880f, 0x05fc, 0x4d3b, \
+ 0xa0, 0x06, 0x74, 0x3f, 0x0f, 0x84, 0x91, 0x1e)
#define PARTITION_LINUX_SWAP_GUID \
- EFI_GUID( 0x0657fd6d, 0xa4ab, 0x43c4, \
- 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
+ EFI_GUID(0x0657fd6d, 0xa4ab, 0x43c4, \
+ 0x84, 0xe5, 0x09, 0x33, 0xc8, 0x4b, 0x4f, 0x4f)
#define PARTITION_LINUX_LVM_GUID \
- EFI_GUID( 0xe6d6d379, 0xf507, 0x44c2, \
- 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)
+ EFI_GUID(0xe6d6d379, 0xf507, 0x44c2, \
+ 0xa2, 0x3c, 0x23, 0x8f, 0x2a, 0x3d, 0xf9, 0x28)

typedef struct _gpt_header {
__le64 signature;
@@ -78,7 +78,7 @@ typedef struct _gpt_header {
typedef struct _gpt_entry_attributes {
u64 required_to_function:1;
u64 reserved:47;
- u64 type_guid_specific:16;
+ u64 type_guid_specific:16;
} __packed gpt_entry_attributes;

typedef struct _gpt_entry {
@@ -87,7 +87,7 @@ typedef struct _gpt_entry {
__le64 starting_lba;
__le64 ending_lba;
gpt_entry_attributes attributes;
- __le16 partition_name[72/sizeof(__le16)];
+ __le16 partition_name[72 / sizeof(__le16)];
} __packed gpt_entry;

typedef struct _gpt_mbr_record {
@@ -103,7 +103,6 @@ typedef struct _gpt_mbr_record {
__le32 size_in_lba; /* used by EFI - size of pt in LBA */
} __packed gpt_mbr_record;

-
typedef struct _legacy_mbr {
u8 boot_code[440];
__le32 unique_mbr_signature;
--
2.43.0