| Message ID | 1426023658-6721-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
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>
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?
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 :)
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>
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!
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