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 <w...@denx.de> wrote:
> Dear Ben Gardiner,
>
> In message <1278366212-24023-3-git-send-email-bengardi...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
