[U-Boot] [PATCH v3 2/4] mtdparts: show net size in mtdparts list

Ben Gardiner
2010-08-09T14:45:42Z

Hi Wolfgang,

Thank you for the review comments. Could you give me some more details
on how you would like the following resolved?

On Sat, Aug 7, 2010 at 4:08 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Ben Gardiner,
>
> In message <1278366212-24023-3-git-send-email-bengardiner@nanometrics.ca> you wrote:
>> +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES)
>> + ? ? ? ? ? ? if (get_mtd_info(dev->id->type, dev->id->num, &mtd))
>> + ? ? ? ? ? ? ? ? ? ? return;
>> +#endif
>>
>> ? ? ? ? ? ? ? /* list partitions for given device */
>> ? ? ? ? ? ? ? part_num = 0;
>> ? ? ? ? ? ? ? list_for_each(pentry, &dev->parts) {
>> ? ? ? ? ? ? ? ? ? ? ? part = list_entry(pentry, struct part_info, link);
>> - ? ? ? ? ? ? ? ? ? ? printf("%2d: %-20s0x%08x\t0x%08x\t%d\n",
>> +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES)
>> + ? ? ? ? ? ? ? ? ? ? net_size = net_part_size(mtd, part);
>> +#endif
>> + ? ? ? ? ? ? ? ? ? ? printf("%2d: %-20s0x%08x\t"
>> +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "0x%08x%s\t"
>> +#endif
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "0x%08x\t%d\n",
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? part_num, part->name, part->size,
>> +#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? net_size,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? part->size == net_size ? " " : " (!)",
>> +#endif
>
> This is way too much #ifdef's here. Please separate the code and use a
> single #ifdef only.

Would it be acceptable to do something like the following?

static void print_partition_table(...)
{
#if defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES)
...
#else /* !defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) */
...
#endif /* defined(CONFIG_CMD_MTDPARTS_SHOW_NET_SIZES) */
}

/**
* Format and print out a partition list for each device from global device
* list.
*/
static void list_partitions(void)
{
...

print_partition_table(...)

/* current_mtd_dev is not NULL only when we have non empty device list */
if (current_mtd_dev) {

...

puts("mtdparts: ");
puts(mtdparts_default ? mtdparts_default : "none");
puts("\n");
}

Best Regards,

Ben Gardiner

---
Nanometrics Inc.