From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Wed, 26 Jun 2013 14:05:15 -0600
Subject: [U-Boot] [PATCH v2 4/5] env: Allow environment files to use the
 C preprocessor
In-Reply-To: <1372106765-18401-5-git-send-email-sjg@chromium.org>
References: <1372106765-18401-1-git-send-email-sjg@chromium.org>
	<1372106765-18401-5-git-send-email-sjg@chromium.org>
Message-ID: <51CB497B.8030404@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 06/24/2013 02:46 PM, Simon Glass wrote:
> In many cases environment variables need access to the U-Boot CONFIG
> variables to select different options. Enable this so that the environment
> scripts can be as useful as the ones currently in the board config files.

The addition of += seems like a separate change to enabling
pre-processing, but I guess they're both simple enough and this is a new
features, perhaps it's not a big deal.

> diff --git a/Makefile b/Makefile

>  $(obj)include/generated/environment.in: $(obj)include/generated/autoconf.mk.base \
>  		$(wildcard $(ENV_FILE))
>  	if [ -f "$(ENV_FILE)" ]; then \
> -		cat $(ENV_FILE) >$@ ; \
> +		$(CPP) -P $(CFLAGS) -x assembler-with-cpp -D__ASSEMBLY__ \
> +			-include $(obj)include/config.h $(ENV_FILE) -o $@; \

I guess -undef doesn't make sense here, since config.h could well rely
on standard pre-defined macros.

Does it make sense to -D __UBOOT_CONFIG__ rather than, or in addition
to, -D __ASSEMBLY__ here, so headers can tell what they're being
included for? The series I sent for dtc+cpp did -D __DTS__ for similar
reasons.

> diff --git a/tools/scripts/env2string.awk b/tools/scripts/env2string.awk

> +		# Deal with +=
> +		if (match(var, "(.*)[+]$", var_arr)) {
> +			var = var_arr[1]
> +			env = vars[var] env
> +		}

Does this work if you write just:

foo+=bar

rather than:

foo=
foo+=bar

It might be worth allowing the former syntax, so you don't need to
explicitly assign empty values before a sequence of ifdef'd +=
operations. If the above blows up, at least a descriptive error message
might be useful.