* [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling
@ 2015-03-10 21:40 Stephen Warren
2015-03-11 15:51 ` Tom Rini
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stephen Warren @ 2015-03-10 21:40 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
This should make it more clear why there appear to be C pre-processor
symbols in the file that contain mixed case. They're really error
messages.
Suggested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
include/config_distro_bootcmd.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index 07a0b3b23472..73f093f9eaf5 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -10,6 +10,22 @@
#ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
#define _CONFIG_CMD_DISTRO_BOOTCMD_H
+/*
+ * A note on error handling: It is possible for BOOT_TARGET_DEVICES to
+ * reference a device that is not enabled in the U-Boot configuration, e.g.
+ * it may include MMC in the list without CONFIG_CMD_MMC being enabled. Given
+ * that BOOT_TARGET_DEVICES is a macro that's expanded by the C pre-processor
+ * at compile time, it's not possible to detect and report such problems via
+ * a simple #ifdef/#error combination. Still, the code needs to report errors.
+ * The best way I've found to do this is to make BOOT_TARGET_DEVICES expand to
+ * reference a non-existent symbol, and have the name of that symbol encode
+ * the error message. Consequently, this file contains references to e.g.
+ * BOOT_TARGET_DEVICES_references_MMC_without_CONFIG_CMD_MMC. Given the
+ * prevalence of capitals here, this looks like a pre-processor macro and
+ * hence seems like it should be all capitals, but it's really an error
+ * message that includes some other pre-processor symbols in the text.
+ */
+
/* We need the part command */
#define CONFIG_PARTITION_UUIDS
#define CONFIG_CMD_PART
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling
2015-03-10 21:40 [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling Stephen Warren
@ 2015-03-11 15:51 ` Tom Rini
2015-03-11 16:22 ` Stephen Warren
2015-03-11 18:42 ` Simon Glass
2015-03-15 21:51 ` [U-Boot] " Tom Rini
2 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2015-03-11 15:51 UTC (permalink / raw)
To: u-boot
On Tue, Mar 10, 2015 at 03:40:58PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This should make it more clear why there appear to be C pre-processor
> symbols in the file that contain mixed case. They're really error
> messages.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/0ef25192/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling
2015-03-11 15:51 ` Tom Rini
@ 2015-03-11 16:22 ` Stephen Warren
2015-03-11 17:35 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2015-03-11 16:22 UTC (permalink / raw)
To: u-boot
On 03/11/2015 09:51 AM, Tom Rini wrote:
> On Tue, Mar 10, 2015 at 03:40:58PM -0600, Stephen Warren wrote:
>
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> This should make it more clear why there appear to be C pre-processor
>> symbols in the file that contain mixed case. They're really error
>> messages.
>>
>> Suggested-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
Thanks. It looks like you usually apply the patches to this file rather
than acking it for someone else to take. Was your reviewed-by just a
hint you're waiting for e.g. Simon to review it too?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling
2015-03-11 16:22 ` Stephen Warren
@ 2015-03-11 17:35 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2015-03-11 17:35 UTC (permalink / raw)
To: u-boot
On Wed, Mar 11, 2015 at 10:22:23AM -0600, Stephen Warren wrote:
> On 03/11/2015 09:51 AM, Tom Rini wrote:
> >On Tue, Mar 10, 2015 at 03:40:58PM -0600, Stephen Warren wrote:
> >
> >>From: Stephen Warren <swarren@nvidia.com>
> >>
> >>This should make it more clear why there appear to be C pre-processor
> >>symbols in the file that contain mixed case. They're really error
> >>messages.
> >>
> >>Suggested-by: Simon Glass <sjg@chromium.org>
> >>Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >
> >Reviewed-by: Tom Rini <trini@konsulko.com>
>
> Thanks. It looks like you usually apply the patches to this file
> rather than acking it for someone else to take. Was your reviewed-by
> just a hint you're waiting for e.g. Simon to review it too?
Yes, thanks :)
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/50911880/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling
2015-03-10 21:40 [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling Stephen Warren
2015-03-11 15:51 ` Tom Rini
@ 2015-03-11 18:42 ` Simon Glass
2015-03-15 21:51 ` [U-Boot] " Tom Rini
2 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2015-03-11 18:42 UTC (permalink / raw)
To: u-boot
On 10 March 2015 at 15:40, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This should make it more clear why there appear to be C pre-processor
> symbols in the file that contain mixed case. They're really error
> messages.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> include/config_distro_bootcmd.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
> index 07a0b3b23472..73f093f9eaf5 100644
> --- a/include/config_distro_bootcmd.h
> +++ b/include/config_distro_bootcmd.h
> @@ -10,6 +10,22 @@
> #ifndef _CONFIG_CMD_DISTRO_BOOTCMD_H
> #define _CONFIG_CMD_DISTRO_BOOTCMD_H
>
> +/*
> + * A note on error handling: It is possible for BOOT_TARGET_DEVICES to
> + * reference a device that is not enabled in the U-Boot configuration, e.g.
> + * it may include MMC in the list without CONFIG_CMD_MMC being enabled. Given
> + * that BOOT_TARGET_DEVICES is a macro that's expanded by the C pre-processor
> + * at compile time, it's not possible to detect and report such problems via
> + * a simple #ifdef/#error combination. Still, the code needs to report errors.
> + * The best way I've found to do this is to make BOOT_TARGET_DEVICES expand to
> + * reference a non-existent symbol, and have the name of that symbol encode
> + * the error message. Consequently, this file contains references to e.g.
> + * BOOT_TARGET_DEVICES_references_MMC_without_CONFIG_CMD_MMC. Given the
> + * prevalence of capitals here, this looks like a pre-processor macro and
> + * hence seems like it should be all capitals, but it's really an error
> + * message that includes some other pre-processor symbols in the text.
> + */
> +
> /* We need the part command */
> #define CONFIG_PARTITION_UUIDS
> #define CONFIG_CMD_PART
Very clear thank you.
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] config_distro_bootcmd.h: add note on error handling
2015-03-10 21:40 [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling Stephen Warren
2015-03-11 15:51 ` Tom Rini
2015-03-11 18:42 ` Simon Glass
@ 2015-03-15 21:51 ` Tom Rini
2 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2015-03-15 21:51 UTC (permalink / raw)
To: u-boot
On Tue, Mar 10, 2015 at 03:40:58PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> This should make it more clear why there appear to be C pre-processor
> symbols in the file that contain mixed case. They're really error
> messages.
>
> Suggested-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150315/178410f9/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-15 21:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-10 21:40 [U-Boot] [PATCH] config_distro_bootcmd.h: add note on error handling Stephen Warren
2015-03-11 15:51 ` Tom Rini
2015-03-11 16:22 ` Stephen Warren
2015-03-11 17:35 ` Tom Rini
2015-03-11 18:42 ` Simon Glass
2015-03-15 21:51 ` [U-Boot] " Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox