From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Althoefer <stefan.althoefer@web.de>
Date: Sun, 07 Dec 2008 19:54:14 +0100
Subject: [U-Boot] [PATCH] common: nvedit to protect additional ethernet
 addresses Part 1/1
In-Reply-To: <20081207162658.62FC1834B020@gemini.denx.de>
References: <4937cd00.hX/ek4tggr3wX3a8%stefan.althoefer@web.de>	<49384204.1070901@ge.com>
	<ghc625$k37$1@ger.gmane.org>	<ghgj6n$sen$1@ger.gmane.org>
	<20081207162658.62FC1834B020@gemini.denx.de>
Message-ID: <ghh632$ims$1@ger.gmane.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

Wolfgang Denk schrieb:
> Dear Stefan Althoefer,
> 
> In message <ghgj6n$sen$1@ger.gmane.org> you wrote:
>> From fdeee62f0902b25be1a2a6bf52fb714b0f4f9e59 Mon Sep 17 00:00:00 2001
>> From: Stefan Althoefer <stefan.althoefer@web.de>
>> Date: Sun, 7 Dec 2008 14:17:08 +0100
>> Subject: [PATCH] common: nvedit to protect additional ethernet addresses
>>
>> This adds "eth[0-9]+addr" to the protected
>> environment variables that can only be written once.
>>
>> Code for detecting protected variables was restructured.
>>
>> Signed-off-by: Stefan Althoefer <stefan.althoefer@web.de>
> ...
>> @@ -181,18 +186,31 @@ int _do_setenv (int flag, int argc, char *argv[])
>>  		 * Ethernet Address and serial# can be set only once,
>>  		 * ver is readonly.
>>  		 */
>> -		if (
>> +		protected = 0;
>>  #ifdef CONFIG_HAS_UID
>>  		/* Allow serial# forced overwrite with 0xdeaf4add flag */
>> -		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
>> +		if ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add))
>>  #else
>> -		    (strcmp (name, "serial#") == 0) ||
>> +		if (strcmp (name, "serial#") == 0)
>>  #endif
>> -		    ((strcmp (name, "ethaddr") == 0)
>> +			protected = 1;
> 
> Here we already know that the variable is "serial#", so it cannot be
> any of the "eth*addr" variables.
> 
>> +		if (strcmp (name, "ethaddr") == 0)
>>  #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
>> -		     && (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)
>> +			/* Allow "ethaddr" overwrite to change pre-configured address */
>> +			if (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)
>>  #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
>> -		    ) ) {
>> +				protected = 1;
>> +
>> +		/* "eth[0-9]+addr" is always protected */
>> +		if (strncmp (name, "eth", 3) == 0) {
>> +			ethnum = simple_strtoul (name+3, &s, 10);
>> +			if (s != name + 3)
>> +				if (strcmp (s, "addr") == 0)
>> +					protected = 1;
>> +		}
> 
> Then why do we continue to test for these impossible cases? It's just
> wasting CPU cycles.
> 
> Best regards,
> 
> Wolfgang Denk
> 

You argue that the code should have a couple of hard to read else cases?

--- Stefan