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. http://www.nanometrics.ca