Skip to contentSkip to conversation

cosmetic: Replace __FUNCTION__ with __func__ in part_efi.c

PATCH New Doug Anderson 4 participants 12 messages Oct 19, 2011 – Oct 27, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 cosmetic: Replace __FUNCTION__ with __func__ in part_efi.c

This makes checkpatch happy.

---
 disk/part_efi.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)
disk/part_efi.c +14 -14
index 0a513c6..6a139a9 100644
@@ -125,17 +125,17 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 	int i = 0;
 
 	if (!dev_desc) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
 		return;
 	}
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
 			 &(gpt_head), pgpt_pte) != 1) {
-		printf("%s: *** ERROR: Invalid GPT ***\n", __FUNCTION__);
+		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return;
 	}
 
-	debug("%s: gpt-entry at 0x%08X\n", __FUNCTION__, (unsigned int)*pgpt_pte);
+	debug("%s: gpt-entry at 0x%08X\n", __func__, (unsigned int)*pgpt_pte);
 
 	printf("Part\tName\t\t\tStart LBA\tEnd LBA\n");
 	for (i = 0; i < le32_to_int(gpt_head.num_partition_entries); i++) {
@@ -152,7 +152,7 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 
 	/* Remember to free pte */
 	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __FUNCTION__);
+		debug("%s: Freeing pgpt_pte\n", __func__);
 		free(*pgpt_pte);
 	}
 	return;
@@ -166,14 +166,14 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 
 	/* "part" argument must be@least 1 */
 	if (!dev_desc || !info || part < 1) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
 		return -1;
 	}
 
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
 			&(gpt_head), pgpt_pte) != 1) {
-		printf("%s: *** ERROR: Invalid GPT ***\n", __FUNCTION__);
+		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return -1;
 	}
 
@@ -188,12 +188,12 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 			print_efiname(&(*pgpt_pte)[part - 1]));
 	sprintf((char *)info->type, "U-Boot");
 
-	debug("%s: start 0x%lX, size 0x%lX, name %s", __FUNCTION__,
+	debug("%s: start 0x%lX, size 0x%lX, name %s", __func__,
 		info->start, info->size, info->name);
 
 	/* Remember to free pte */
 	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __FUNCTION__);
+		debug("%s: Freeing pgpt_pte\n", __func__);
 		free(*pgpt_pte);
 	}
 	return 0;
@@ -271,7 +271,7 @@ static int is_gpt_valid(block_dev_desc_t * dev_desc, unsigned long long lba,
 	unsigned long long lastlba;
 
 	if (!dev_desc || !pgpt_head) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
 		return 0;
 	}
 
@@ -375,14 +375,14 @@ static gpt_entry *alloc_read_gpt_entries(block_dev_desc_t * dev_desc,
 	gpt_entry *pte = NULL;
 
 	if (!dev_desc || !pgpt_head) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
 		return NULL;
 	}
 
 	count = le32_to_int(pgpt_head->num_partition_entries) *
 		le32_to_int(pgpt_head->sizeof_partition_entry);
 
-	debug("%s: count = %lu * %lu = %u\n", __FUNCTION__,
+	debug("%s: count = %lu * %lu = %u\n", __func__,
 		le32_to_int(pgpt_head->num_partition_entries),
 		le32_to_int(pgpt_head->sizeof_partition_entry), count);
 
@@ -393,7 +393,7 @@ static gpt_entry *alloc_read_gpt_entries(block_dev_desc_t * dev_desc,
 
 	if (count == 0 || pte == NULL) {
 		printf("%s: ERROR: Can't allocate 0x%X bytes for GPT Entries\n",
-			__FUNCTION__, count);
+			__func__, count);
 		return NULL;
 	}
 
@@ -421,7 +421,7 @@ static int is_pte_valid(gpt_entry * pte)
 	efi_guid_t unused_guid;
 
 	if (!pte) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
 		return 0;
 	}
 
@@ -433,7 +433,7 @@ static int is_pte_valid(gpt_entry * pte)
 	if (memcmp(pte->partition_type_guid.b, unused_guid.b,
 		sizeof(unused_guid.b)) == 0) {
 
-		debug("%s: Found an unused PTE GUID at 0x%08X\n", __FUNCTION__,
+		debug("%s: Found an unused PTE GUID at 0x%08X\n", __func__,
 		(unsigned int)pte);
 
 		return 0;
Signature · 1 line
1.7.3.1
#1 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

Code was setting **pgpt_pte == NULL, which meant that the pointer to the gpt_pte would be stored at RAM address 00000000. This 'worked' on T20 (SDRAM starts @ 0x00000000), but hung gpt/EFI access on T30 (SDRAM starts @ 0x80000000).

---
This was inspired by a change that I obtained through Tom Warren at nVidia.

 disk/part_efi.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)
disk/part_efi.c +18 -18
index 6a139a9..08bd560 100644
@@ -121,7 +121,7 @@ static char *print_efiname(gpt_entry *pte)
 void print_part_efi(block_dev_desc_t * dev_desc)
 {
 	gpt_header gpt_head;
-	gpt_entry **pgpt_pte = NULL;
+	gpt_entry *gpt_pte = NULL;
 	int i = 0;
 
 	if (!dev_desc) {
@@ -130,30 +130,30 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 	}
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			 &(gpt_head), pgpt_pte) != 1) {
+			 &(gpt_head), &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return;
 	}
 
-	debug("%s: gpt-entry at 0x%08X\n", __func__, (unsigned int)*pgpt_pte);
+	debug("%s: gpt-entry at 0x%p\n", __func__, gpt_pte);
 
 	printf("Part\tName\t\t\tStart LBA\tEnd LBA\n");
 	for (i = 0; i < le32_to_int(gpt_head.num_partition_entries); i++) {
 
-		if (is_pte_valid(&(*pgpt_pte)[i])) {
+		if (is_pte_valid(&gpt_pte[i])) {
 			printf("%3d\t%-18s\t0x%08llX\t0x%08llX\n", (i + 1),
-				print_efiname(&(*pgpt_pte)[i]),
-				le64_to_int((*pgpt_pte)[i].starting_lba),
-				le64_to_int((*pgpt_pte)[i].ending_lba));
+				print_efiname(&gpt_pte[i]),
+				le64_to_int(gpt_pte[i].starting_lba),
+				le64_to_int(gpt_pte[i].ending_lba));
 		} else {
 			break;	/* Stop at the first non valid PTE */
 		}
 	}
 
 	/* Remember to free pte */
-	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __func__);
-		free(*pgpt_pte);
+	if (gpt_pte != NULL) {
+		debug("%s: Freeing gpt_pte\n", __func__);
+		free(gpt_pte);
 	}
 	return;
 }
@@ -162,7 +162,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 				disk_partition_t * info)
 {
 	gpt_header gpt_head;
-	gpt_entry **pgpt_pte = NULL;
+	gpt_entry *gpt_pte = NULL;
 
 	/* "part" argument must be@least 1 */
 	if (!dev_desc || !info || part < 1) {
@@ -172,29 +172,29 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			&(gpt_head), pgpt_pte) != 1) {
+			&(gpt_head), &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return -1;
 	}
 
 	/* The ulong casting limits the maximum disk size to 2 TB */
-	info->start = (ulong) le64_to_int((*pgpt_pte)[part - 1].starting_lba);
+	info->start = (ulong) le64_to_int(gpt_pte[part - 1].starting_lba);
 	/* The ending LBA is inclusive, to calculate size, add 1 to it */
-	info->size = ((ulong)le64_to_int((*pgpt_pte)[part - 1].ending_lba) + 1)
+	info->size = ((ulong)le64_to_int(gpt_pte[part - 1].ending_lba) + 1)
 		     - info->start;
 	info->blksz = GPT_BLOCK_SIZE;
 
 	sprintf((char *)info->name, "%s",
-			print_efiname(&(*pgpt_pte)[part - 1]));
+			print_efiname(&gpt_pte[part - 1]));
 	sprintf((char *)info->type, "U-Boot");
 
 	debug("%s: start 0x%lX, size 0x%lX, name %s", __func__,
 		info->start, info->size, info->name);
 
 	/* Remember to free pte */
-	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __func__);
-		free(*pgpt_pte);
+	if (gpt_pte != NULL) {
+		debug("%s: Freeing gpt_pte\n", __func__);
+		free(gpt_pte);
 	}
 	return 0;
 }
Signature · 1 line
1.7.3.1
#2 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

disk/part_efi.c void print_part_efi(block_dev_desc_t * dev_desc) · line 138

Quoted 2 lines · On Wednesday 19 October 2011 14:04:47 Doug Anderson wrote:
disk/part_efi.c +1 -1
-	debug("%s: gpt-entry at 0x%08X\n", __func__, (unsigned int)*pgpt_pte);
+	debug("%s: gpt-entry at 0x%p\n", __func__, gpt_pte);

i like %x->%p, but i think this change is incorrect. doesn't %p output a leading 0x already ?

disk/part_efi.c void print_part_efi(block_dev_desc_t * dev_desc) · line 157

Quoted 8 lines
disk/part_efi.c +3 -3
 	/* Remember to free pte */
-	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __func__);
-		free(*pgpt_pte);
+	if (gpt_pte != NULL) {
+		debug("%s: Freeing gpt_pte\n", __func__);
+		free(gpt_pte);
 	}

useless if() check. free(NULL) works fine. change it to always call free(). there are a few more like this below.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111019/069dd85b/attachment.pgp

#3 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

Code was setting **pgpt_pte == NULL, which meant that the pointer to the gpt_pte would be stored at RAM address 00000000. This 'worked' on T20 (SDRAM starts @ 0x00000000), but hung gpt/EFI access on T30 (SDRAM starts @ 0x80000000).

---
v2: No change to patch 1/2 (not reposted); patch 2/2 updated to address Mike Frysinger's comments. Note that %p currently doesn't include the "0x" prefix in u-boot, but I expect that it may someday (since it does in gcc) so I've gone ahead and removed the prefix.

 disk/part_efi.c |   38 +++++++++++++++-----------------------
 1 files changed, 15 insertions(+), 23 deletions(-)
disk/part_efi.c +15 -23
index 6a139a9..ee3ff86 100644
@@ -121,7 +121,7 @@ static char *print_efiname(gpt_entry *pte)
 void print_part_efi(block_dev_desc_t * dev_desc)
 {
 	gpt_header gpt_head;
-	gpt_entry **pgpt_pte = NULL;
+	gpt_entry *gpt_pte = NULL;
 	int i = 0;
 
 	if (!dev_desc) {
@@ -130,31 +130,28 @@ void print_part_efi(block_dev_desc_t * dev_desc)
 	}
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			 &(gpt_head), pgpt_pte) != 1) {
+			 &(gpt_head), &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return;
 	}
 
-	debug("%s: gpt-entry at 0x%08X\n", __func__, (unsigned int)*pgpt_pte);
+	debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
 
 	printf("Part\tName\t\t\tStart LBA\tEnd LBA\n");
 	for (i = 0; i < le32_to_int(gpt_head.num_partition_entries); i++) {
 
-		if (is_pte_valid(&(*pgpt_pte)[i])) {
+		if (is_pte_valid(&gpt_pte[i])) {
 			printf("%3d\t%-18s\t0x%08llX\t0x%08llX\n", (i + 1),
-				print_efiname(&(*pgpt_pte)[i]),
-				le64_to_int((*pgpt_pte)[i].starting_lba),
-				le64_to_int((*pgpt_pte)[i].ending_lba));
+				print_efiname(&gpt_pte[i]),
+				le64_to_int(gpt_pte[i].starting_lba),
+				le64_to_int(gpt_pte[i].ending_lba));
 		} else {
 			break;	/* Stop at the first non valid PTE */
 		}
 	}
 
 	/* Remember to free pte */
-	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __func__);
-		free(*pgpt_pte);
-	}
+	free(gpt_pte);
 	return;
 }
 
@@ -162,7 +159,7 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 				disk_partition_t * info)
 {
 	gpt_header gpt_head;
-	gpt_entry **pgpt_pte = NULL;
+	gpt_entry *gpt_pte = NULL;
 
 	/* "part" argument must be@least 1 */
 	if (!dev_desc || !info || part < 1) {
@@ -172,30 +169,27 @@ int get_partition_info_efi(block_dev_desc_t * dev_desc, int part,
 
 	/* This function validates AND fills in the GPT header and PTE */
 	if (is_gpt_valid(dev_desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-			&(gpt_head), pgpt_pte) != 1) {
+			&(gpt_head), &gpt_pte) != 1) {
 		printf("%s: *** ERROR: Invalid GPT ***\n", __func__);
 		return -1;
 	}
 
 	/* The ulong casting limits the maximum disk size to 2 TB */
-	info->start = (ulong) le64_to_int((*pgpt_pte)[part - 1].starting_lba);
+	info->start = (ulong) le64_to_int(gpt_pte[part - 1].starting_lba);
 	/* The ending LBA is inclusive, to calculate size, add 1 to it */
-	info->size = ((ulong)le64_to_int((*pgpt_pte)[part - 1].ending_lba) + 1)
+	info->size = ((ulong)le64_to_int(gpt_pte[part - 1].ending_lba) + 1)
 		     - info->start;
 	info->blksz = GPT_BLOCK_SIZE;
 
 	sprintf((char *)info->name, "%s",
-			print_efiname(&(*pgpt_pte)[part - 1]));
+			print_efiname(&gpt_pte[part - 1]));
 	sprintf((char *)info->type, "U-Boot");
 
 	debug("%s: start 0x%lX, size 0x%lX, name %s", __func__,
 		info->start, info->size, info->name);
 
 	/* Remember to free pte */
-	if (*pgpt_pte != NULL) {
-		debug("%s: Freeing pgpt_pte\n", __func__);
-		free(*pgpt_pte);
-	}
+	free(gpt_pte);
 	return 0;
 }
 
@@ -349,9 +343,7 @@ static int is_gpt_valid(block_dev_desc_t * dev_desc, unsigned long long lba,
 			le32_to_int(pgpt_head->partition_entry_array_crc32),
 			calc_crc32);
 
-		if (*pgpt_pte != NULL) {
-			free(*pgpt_pte);
-		}
+		free(*pgpt_pte);
 		return 0;
 	}
 
Signature · 1 line
1.7.3.1
#4 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

Quoted 4 lines · On Wednesday 19 October 2011 15:47:31 Doug Anderson wrote:

v2: No change to patch 1/2 (not reposted); patch 2/2 updated to address Mike Frysinger's comments. Note that %p currently doesn't include the "0x" prefix in u-boot, but I expect that it may someday (since it does in gcc) so I've gone ahead and removed the prefix.

%#p gives you the prefix under u-boot, but gcc warns about it since it assumes that %p always includes the 0x prefix :).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111019/13b27fa7/attachment.pgp

#5 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

Quoted 4 lines · Mike Frysinger

%#p gives you the prefix under u-boot, but gcc warns about it since it assumes
that %p always includes the 0x prefix :).

Ah, makes sense. I'm just going to leave it as %p without the 0x. That sound OK? I assume that the patch looks OK to you now?

Thanks for your review!

-Doug

P.S. apologies for forgetting to change the subject to "PATCH v2". I realized my mistake after submitting. :(

#6 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

Quoted 7 lines · On Wednesday 19 October 2011 15:59:27 Doug Anderson wrote:
Quoted 3 lines · Mike Frysinger

%#p gives you the prefix under u-boot, but gcc warns about it since it assumes
that %p always includes the 0x prefix :).

Ah, makes sense. I'm just going to leave it as %p without the 0x. That sound OK? I assume that the patch looks OK to you now?

yeah, that's fine. i think we'll want to update u-boot's printf implementation to prefix 0x with %p to match what everyone else is doing.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111019/14ab6d1c/attachment.pgp

#7 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

In reply to Doug Anderson

Dear Doug Anderson,

Quoted 7 lines · In message <[email protected]> you wrote:

Code was setting **pgpt_pte == NULL, which meant that the pointer to the gpt_pte would be stored at RAM address 00000000. This 'worked' on T20 (SDRAM starts @ 0x00000000), but hung gpt/EFI access on T30 (SDRAM starts @ 0x80000000).

Quoted 8 lines

---
v2: No change to patch 1/2 (not reposted); patch 2/2 updated to address Mike Frysinger's comments. Note that %p currently doesn't include the "0x" prefix in u-boot, but I expect that it may someday (since it does in gcc) so I've gone ahead and removed the prefix.

 disk/part_efi.c |   38 +++++++++++++++-----------------------
 1 files changed, 15 insertions(+), 23 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I have a very small mind and must live with it.    -- Edsger Dijkstra
#8 lore raw

PATCH 2/2 disk: part_efi: fix **pgpt_pte == NULL

In reply to Doug Anderson
Quoted 19 lines · Doug Anderson

Code was setting **pgpt_pte == NULL, which meant that the pointer to the gpt_pte would be stored at RAM address 00000000. This 'worked' on T20 (SDRAM starts @ 0x00000000), but hung gpt/EFI access on T30 (SDRAM starts @ 0x80000000).

---
This was inspired by a change that I obtained through Tom Warren at nVidia.

  disk/part_efi.c |   36 ++++++++++++++++++------------------
  1 files changed, 18 insertions(+), 18 deletions(-)
disk/part_efi.c +0 -0
index 6a139a9..08bd560 100644

disk/part_efi.c static char *print_efiname(gpt_entry *pte) · line 125

Quoted 6 lines
disk/part_efi.c +1 -1
@@ -121,7 +121,7 @@ static char *print_efiname(gpt_entry *pte)
  void print_part_efi(block_dev_desc_t * dev_desc)
  {
  	gpt_header gpt_head;
-	gpt_entry **pgpt_pte = NULL;
+	gpt_entry *gpt_pte = NULL;
  	int i = 0;

ACK, good find!

- Richard

#10 lore raw

PATCH 1/2 cosmetic: Replace __FUNCTION__ with __func__ in part_efi.c

In reply to Doug Anderson
Quoted 12 lines · Doug Anderson

This makes checkpatch happy.

---
  disk/part_efi.c |   28 ++++++++++++++--------------
  1 files changed, 14 insertions(+), 14 deletions(-)
disk/part_efi.c +0 -0
index 0a513c6..6a139a9 100644

disk/part_efi.c void print_part_efi(block_dev_desc_t * dev_desc) · line 129

Quoted 6 lines
disk/part_efi.c +1 -1
@@ -125,17 +125,17 @@ void print_part_efi(block_dev_desc_t * dev_desc)
  	int i = 0;
 
  	if (!dev_desc) {
-		printf("%s: Invalid Argument(s)\n", __FUNCTION__);
+		printf("%s: Invalid Argument(s)\n", __func__);
  		return;

<snip>
Hi Doug,

ACK, thanks for the cleanup!

- Richard

#11 lore raw

PATCH 1/2 cosmetic: Replace __FUNCTION__ with __func__ in part_efi.c

In reply to Doug Anderson

Dear Doug Anderson,

Quoted 6 lines · In message <[email protected]> you wrote:

This makes checkpatch happy.

---
 disk/part_efi.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Just think of a computer as hardware you can program."
- Nigel de la Tierre