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

[Ben Gardiner]

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 at denx.de> wrote:
> Dear Ben Gardiner,
>
> In message <1278366212-24023-3-git-send-email-bengardiner at 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.
http://www.nanometrics.ca