common: nvedit to protect additional ethernet addresses Part 1/1

PATCH New stefan.althoefer at web.de 5 participants 17 messages Dec 4, 2008 – Dec 16, 2008

lore

Share & tools

lore

stefan.althoefer at web.de

author

2008-12-04 12:28 UTC #0 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

[PATCH] common: nvedit to protect additional ethernet addresses

This patch adds "ethaddr1" and "ethaddr2" to the protected environment variables that can only be written once.

----
The patch is against "latest" u-boot git-repository

Please be patient if style of submission or patches are offending.

Signed-off-by: stefan.althoefer at web.de (as at janz.de)
----

common/cmd_nvedit.c renamed from /common/cmd_nvedit.c +2 -0

diff -uprN u-boot-orig//common/cmd_nvedit.c u-boot/common/cmd_nvedit.c
@@ -188,6 +188,8 @@ int _do_setenv (int flag, int argc, char
 #else
 		    (strcmp (name, "serial#") == 0) ||
 #endif
+		    (strcmp (name, "ethaddr1") == 0) ||
+		    (strcmp (name, "ethaddr2") == 0) ||
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
 		     && (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)

Jerry Van Baren

2008-12-04 12:46 UTC #1 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Quoted 18 lines · stefan.althoefer at web.de

[PATCH] common: nvedit to protect additional ethernet addresses
This patch adds "ethaddr1" and "ethaddr2" to the protected environment variables that can only be written once.
----
The patch is against "latest" u-boot git-repository
Please be patient if style of submission or patches are offending.
Signed-off-by: stefan.althoefer at web.de (as at janz.de)
----
diff -uprN u-boot-orig//common/cmd_nvedit.c u-boot/common/cmd_nvedit.c

common/cmd_nvedit.c int _do_setenv (int flag, int argc, char · line 195

Quoted 8 lines

common/cmd_nvedit.c renamed from /common/cmd_nvedit.c +2 -0

@@ -188,6 +188,8 @@ int _do_setenv (int flag, int argc, char
 #else
 		    (strcmp (name, "serial#") == 0) ||
 #endif
+		    (strcmp (name, "ethaddr1") == 0) ||
+		    (strcmp (name, "ethaddr2") == 0) ||
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
 		     && (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)

Would it be better to use "strncmp (name, "ethaddr", 7)" instead of adding all possible enumerations of ethaddr[0-9]*? Using strncmp() will match any ethaddr.* variable (a wider net than ethaddr[0-9]*, but given our limited environment it shouldn't matter).

gvb

Ben Warren

2008-12-04 17:57 UTC #2 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

In reply to stefan.althoefer at web.de

Quoted 5 lines · stefan.althoefer at web.de

[PATCH] common: nvedit to protect additional ethernet addresses
This patch adds "ethaddr1" and "ethaddr2" to the protected environment variables that can only be written once.

Quoted 8 lines

----
The patch is against "latest" u-boot git-repository
Please be patient if style of submission or patches are offending.
Signed-off-by stefan.althoefer at web.de (as at janz.de)

This needs to be above a '---' line. Also, this format is wrong. It should have a real name and one e-mail address. I recommend using 'git format-patch' for generating patches.

common/cmd_nvedit.c int _do_setenv (int flag, int argc, char · line 194

Quoted 13 lines

----

diff -uprN u-boot-orig//common/cmd_nvedit.c u-boot/common/cmd_nvedit.c

common/cmd_nvedit.c renamed from /common/cmd_nvedit.c +2 -0

@@ -188,6 +188,8 @@ int _do_setenv (int flag, int argc, char
 #else
 		    (strcmp (name, "serial#") == 0) ||
 #endif
+		    (strcmp (name, "ethaddr1") == 0) ||
+		    (strcmp (name, "ethaddr2") == 0) ||
 		    ((strcmp (name, "ethaddr") == 0)

How about this as a more scalable solution: (note that some boards have up to "ethaddr5")

       (strstr(name, "ethaddr") == name)

Quoted 7 lines

common/cmd_nvedit.c renamed from /common/cmd_nvedit.c +0 -0
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
&& (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

regards,
Ben

Jerry Van Baren

2008-12-04 20:48 UTC #3 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

In reply to stefan.althoefer at web.de

(Resent in response to complex, non scalable suggestions: IMHO
strncmp (name, "ethaddr", 7)
is a simple and good solution that covers all known and several unknown cases.)

Quoted 18 lines · stefan.althoefer at web.de

[PATCH] common: nvedit to protect additional ethernet addresses
This patch adds "ethaddr1" and "ethaddr2" to the protected environment variables that can only be written once.
----
The patch is against "latest" u-boot git-repository
Please be patient if style of submission or patches are offending.
Signed-off-by: stefan.althoefer at web.de (as at janz.de)
----
diff -uprN u-boot-orig//common/cmd_nvedit.c u-boot/common/cmd_nvedit.c

common/cmd_nvedit.c int _do_setenv (int flag, int argc, char · line 195

Quoted 8 lines

common/cmd_nvedit.c renamed from /common/cmd_nvedit.c +2 -0

@@ -188,6 +188,8 @@ int _do_setenv (int flag, int argc, char
 #else
 		    (strcmp (name, "serial#") == 0) ||
 #endif
+		    (strcmp (name, "ethaddr1") == 0) ||
+		    (strcmp (name, "ethaddr2") == 0) ||
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
 		     && (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)

gvb

Stefan Althoefer

author

2008-12-05 21:22 UTC #4 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

This patches cmd_nvedit to reject changes for "ethaddr." in addition to "ethaddr" and "serial#". This is intendend to protect changes to additional ethernet addresses (e.g. "ethernet1").

The code was rewritten to be more clear.

Signed-off-by Stefan Althoefer <[email protected]>

---

common/cmd_nvedit.c +20 ~~-11~~

diff -uprN u-boot-orig/common/cmd_nvedit.c u-boot/common/cmd_nvedit.c
@@ -143,6 +143,9 @@ int do_printenv (cmd_tbl_t *cmdtp, int f
 int _do_setenv (int flag, int argc, char *argv[])
 {
 	int   i, len, oldval;
+#ifndef CONFIG_ENV_OVERWRITE
+	int   protected;
+#endif
 	int   console = -1;
 	uchar *env, *nxt = NULL;
 	char *name;

@@ -176,23 +179,29 @@ int _do_setenv (int flag, int argc, char
 	 */
 	if (oldval >= 0) {
 #ifndef CONFIG_ENV_OVERWRITE
+		protected = 0;
 
-		/*
-		 * Ethernet Address and serial# can be set only once,
-		 * ver is readonly.
-		 */
-		if (
+		/* "serial#" is protect unless allowed by 0xdeaf4add flag */
 #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;
+
+		/* "ethaddr" is protected unless allowed by OVERWRITE_ONCE */
+		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)
+			if (strcmp ((char *)env_get_addr(oldval),MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
-		    ) ) {
+				protected = 1;
+
+		/* "ethaddr." is always protected */
+		if (strncmp (name, "ethaddr", 7) == 0)
+			if (strlen (name) == 8)
+				protected = 1;
+
+		if (protected) {
 			printf ("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}

Wolfgang Denk

NAK

2008-12-07 00:43 UTC #5 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Stefan Althoefer,

Quoted 3 lines · In message <[email protected]> you wrote:

This patches cmd_nvedit to reject changes for "ethaddr." in addition to "ethaddr" and "serial#". This is intendend to protect changes to additional ethernet addresses (e.g. "ethernet1").

The patch is bogus, as additional ethernet addrssses are eth1addr, eth2addr, etc. and not ethaddr1, etc.

Also, please don't remove perfectly good comments.

NAK.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The more data I punch in this card,  the lighter it becomes, and the
lower the mailing cost."
                     - Stan Kelly-Bootle, "The Devil's DP Dictionary"

Jerry Van Baren

2008-12-07 02:53 UTC #6 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Quoted 11 lines · Wolfgang Denk

Dear Stefan Althoefer,
Quoted 3 lines · In message <[email protected]> you wrote:
This patches cmd_nvedit to reject changes for "ethaddr." in addition to "ethaddr" and "serial#". This is intendend to protect changes to additional ethernet addresses (e.g. "ethernet1").
The patch is bogus, as additional ethernet addrssses are eth1addr, eth2addr, etc. and not ethaddr1, etc.

Quoted 7 lines

Also, please don't remove perfectly god comments.
NAK.
Best regards,
Wolfgang Denk

Arrgh, I was thinking I was so clever with strncmp() and it turns out I was being clever based on a totally bogus assumption (wrong format). :-( I *hate* it when that happens.

The following should work for eth[0-9]+addr (untested):

	int ethnum;
	:
	:
		/* "eth[0-9]+addr" is always protected */
		if ((sscanf(name, "eth%daddr", &ethnum) == 1) &&
		    (ethnum < MAX_ETH_ADDRS))
			protected = 1;

Notes:
* The "ethaddr" case is handled prior to the above snippet of code.
* I took out the added check "if (strlen (name) == 8)", I'm not sure why that was in there, it would limit us to 10 ethernets. If extra validation is desired, ethnum could be checked to be less than MAX_ETH_ADDRS. On reflection, it seems like a good idea so I added it above.
* This is somewhat better than the strncmp() trick because the sscanf() will only convert digits.

gvb

Ben Warren

2008-12-07 05:47 UTC #7 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Quoted 18 lines · Jerry Van Baren

Quoted 16 lines · Wolfgang Denk
Dear Stefan Althoefer,
Quoted 4 lines · In message <[email protected]> you wrote:
This patches cmd_nvedit to reject changes for "ethaddr." in addition to "ethaddr" and "serial#". This is intendend to protect changes to additional ethernet addresses (e.g. "ethernet1").
The patch is bogus, as additional ethernet addrssses are eth1addr, eth2addr, etc. and not ethaddr1, etc.
Also, please don't remove perfectly god comments.
NAK.

Quoted 10 lines

Quoted 4 lines
Best regards,
Wolfgang Denk
Arrgh, I was thinking I was so clever with strncmp() and it turns out I was being clever based on a totally bogus assumption (wrong format). :-( I *hate* it when that happens.

Uh yeah, I wasn't as clever as you by half, but definitely as bogus :) It's good to know Wolfgang has such keen eyes.

Quoted 10 lines

The following should work for eth[0-9]+addr (untested):

	int ethnum;
	:
	:
		/* "eth[0-9]+addr" is always protected */
		if ((sscanf(name, "eth%daddr", &ethnum) == 1) &&
		    (ethnum < MAX_ETH_ADDRS))
			protected = 1;

Quoted 11 lines

Notes:
* The "ethaddr" case is handled prior to the above snippet of code.
* I took out the added check "if (strlen (name) == 8)", I'm not sure why that was in there, it would limit us to 10 ethernets. If extra validation is desired, ethnum could be checked to be less than MAX_ETH_ADDRS. On reflection, it seems like a good idea so I added it above.
* This is somewhat better than the strncmp() trick because the sscanf() will only convert digits.

Neat, although it's very tempting to pull a Python 3 and say "screw reverse compatibility" and change this naming convention.

Quoted 2 lines

gvb
_________________________

regards,
Ben

Wolfgang Denk

2008-12-07 09:33 UTC #8 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Ben Warren,

Quoted 3 lines · In message <[email protected]> you wrote:

Neat, although it's very tempting to pull a Python 3 and say "screw reverse compatibility" and change this naming convention.

If we'd change it, then my vote wouldbe to change ethaddr into eth0addr :-)

The reasoning for the name (as I see it today [which is different from the time when "ethaddr" was used for the first time]) is "MAC address for the Ethernet interface as it's being called in Linux", i. e. "ethNaddr" means "MAC address of the `ethN' network interface (in Linux)".

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It may be bad manners to talk with your mouth full, but it isn't  too
good either if you speak when your head is empty.

Stefan Althoefer

author

2008-12-07 13:31 UTC #9 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

In reply to Stefan Althoefer

Wolfgang Denk

2008-12-07 16:26 UTC #10 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Stefan Althoefer,

Quoted 11 lines · In message <[email protected]> you wrote:

From fdeee62f0902b25be1a2a6bf52fb714b0f4f9e59 Mon Sep 17 00:00:00 2001
From: Stefan Althoefer <[email protected]>
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 <[email protected]>

...

Quoted 6 lines

common/cmd_nvedit.c +1 -1

@@ -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;

common/cmd_nvedit.c int _do_setenv (int flag, int argc, char *argv[]) · line 196

Quoted 10 lines

common/cmd_nvedit.c +3 -3

 #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.

Quoted 6 lines

common/cmd_nvedit.c +3 -1

+		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 */

common/cmd_nvedit.c int _do_setenv (int flag, int argc, char

Quoted 10 lines

common/cmd_nvedit.c +9 -1

-		    ) ) {
+				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

Signature · 7 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How many seconds are there in a year? If I tell you there are 3.155 x
10^7, you won't even try to remember it. On the other hand, who could
forget that, to within half a percent, pi seconds is a nanocentury.
                                                - Tom Duff, Bell Labs

Stefan Althoefer

author

2008-12-07 18:54 UTC #11 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Quoted 55 lines · Wolfgang Denk

Dear Stefan Althoefer,
Quoted 11 lines · In message <[email protected]> you wrote:
From fdeee62f0902b25be1a2a6bf52fb714b0f4f9e59 Mon Sep 17 00:00:00 2001
From: Stefan Althoefer <[email protected]>
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 <[email protected]>
...
Quoted 6 lines
common/cmd_nvedit.c +1 -1
@@ -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;
common/cmd_nvedit.c int _do_setenv (int flag, int argc, char *argv[]) · line 196
Quoted 10 lines
common/cmd_nvedit.c +3 -3
 #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.
Quoted 6 lines
common/cmd_nvedit.c +3 -1
+		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 */
common/cmd_nvedit.c int _do_setenv (int flag, int argc, char
Quoted 10 lines
common/cmd_nvedit.c +9 -1
-		    ) ) {
+				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.

Quoted 4 lines

Best regards,
Wolfgang Denk

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

--- Stefan

Wolfgang Denk

2008-12-07 21:25 UTC #12 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Stefan Althoefer,

Quoted 2 lines · In message <[email protected]> you wrote:

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

That would be one way to avoid unnecessary tests.

Probably not the most elegant approach, agreed.

There are other options, though.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anyone who isn't confused here doesn't really know what's going on.

Stefan Althoefer

author

2008-12-07 23:52 UTC #13 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Hi Wolfgang Denk

Quoted 10 lines

Dear Stefan Althoefer,
Quoted 1 line · In message <[email protected]> you wrote:
You argue that the code should have a couple of hard to read else cases?
That would be one way to avoid unnecessary tests.
Probably not the most elegant approach, agreed.
There are other options, though.

But your suggested optimizations will only be effective if someone tries to write to "serial#". This is not normally done (attempt can be considered an error).

If access to any nonprotected environment variables is requested (and speed does matter here) then any of the protected cases must be tested.

Even you code:

--------
if( "serial#" )
...
else if( "ethaddr" )
...
else if( "eth[0-9]+addr" )
...
---------

then all the ifs are triggered if you write to "videomode".

--- Stefan

Wolfgang Denk

2008-12-15 22:55 UTC #14 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Stefan Althoefer,

Quoted 10 lines · In message <[email protected]> you wrote:

Quoted 6 lines
That would be one way to avoid unnecessary tests.
Probably not the most elegant approach, agreed.
There are other options, though.
But your suggested optimizations will only be effective if someone tries to write to "serial#". This is not normally done (attempt can be considered an error).

Can it? What make you think so?

There are lots of boards that come fresh out of production with a virgin environment, where setting "serial#" is a perfectly normal thing, and not an error at all.

Quoted 2 lines

If access to any nonprotected environment variables is requested (and speed does matter here) then any of the protected cases must be tested.

Yes, but you can do this in many different ways - more and less efficient ones.

Quoted 11 lines

Even you code:
--------
if( "serial#" )
...
else if( "ethaddr" )
...
else if( "eth[0-9]+addr" )
...
---------

Quoted 1 line

then all the ifs are triggered if you write to "videomode".

Yes, we probably can agree that this is one of the less efficient implementations [I take it as pseudo code, because otherwise all cases would execute the "if" branch.]

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If all you have is a hammer, everything looks like a nail.

Stefan Althoefer

author

2008-12-16 21:34 UTC #15 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

Dear Wolfgang Denx,

Quoted 9 lines

Quoted 3 lines
But your suggested optimizations will only be effective if someone tries to write to "serial#". This is not normally done (attempt can be considered an error).
Can it? What make you think so?
There are lots of boards that come fresh out of production with a virgin environment, where setting "serial#" is a perfectly normal thing, and not an error at all.

Indeed, in the factory it is normal to set the serial number. Any later attempt to "overwrite" it is an error (why the code rejects doing it).

-- Stefan

Jerry Van Baren

2008-12-08 20:14 UTC #16 lore raw

PATCH 1/1 common: nvedit to protect additional ethernet addresses Part 1/1

In reply to Stefan Althoefer

Quoted 5 lines · Stefan Althoefer

From fdeee62f0902b25be1a2a6bf52fb714b0f4f9e59 Mon Sep 17 00:00:00 2001
From: Stefan Althoefer <[email protected]>
Date: Sun, 7 Dec 2008 14:17:08 +0100
Subject: [PATCH] common: nvedit to protect additional ethernet addresses

Quoted 8 lines

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 <[email protected]>
---
This time I left god comments ;-)

I always leave god comments too. People look at my comments and say "oh god!"

:-D

gvb