Skip to contentSkip to conversation

Patch to clean up syntax highlighting

U-Boot-Users Applied Steven A. Falco 5 participants 6 messages Jun 12, 2008 – Jul 5, 2008

lore
Share & tools
lore
#0 lore raw

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c file, because it apparently does not parse C ifdef/else/endif. The following patch does not change the behavior of the code at all, but does allow the editor to properly syntax-highlight the file.

Comments invited.

common/cmd_nvedit.c +3 -2
index 9c5d1fc..1ac91ea 100644
@@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
 		 * Ethernet Address and serial# can be set only once,
 		 * ver is readonly.
 		 */
+		if (
 #ifdef CONFIG_HAS_UID
 		/* Allow serial# forced overwrite with 0xdeaf4add flag */
-		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
+		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
 #else
-		if ( (strcmp (name, "serial#") == 0) ||
+		    (strcmp (name, "serial#") == 0) ||
 #endif
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
#1 lore raw
Quoted 6 lines · Steven A. Falco

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c file, because it apparently does not parse C ifdef/else/endif. The following patch does not change the behavior of the code at all, but does allow the editor to properly syntax-highlight the file.

Comments invited.

OK, you invited comments so I'll be the designated curmudgeon tonight... I'm not wild about uglifying our code to accommodating vim syntax highlighting limitations. Perhaps you can fix the vim syntax highlighting instead?

Quoted 11 lines
common/cmd_nvedit.c +1 -0
index 9c5d1fc..1ac91ea 100644
@@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
 		 * Ethernet Address and serial# can be set only once,
 		 * ver is readonly.
 		 */
+		if (

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

Quoted 10 lines
common/cmd_nvedit.c +2 -2
 #ifdef CONFIG_HAS_UID
 		/* Allow serial# forced overwrite with 0xdeaf4add flag */
-		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
+		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
 #else
-		if ( (strcmp (name, "serial#") == 0) ||
+		    (strcmp (name, "serial#") == 0) ||
 #endif
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)

I probably should confess I don't use syntax highlighting so that probably factors in to my lack of sympathy.

Best regards,
gvb

#2 lore raw
Quoted 38 lines · Jerry Van Baren
Quoted 6 lines · Steven A. Falco

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c file, because it apparently does not parse C ifdef/else/endif. The following patch does not change the behavior of the code at all, but does allow the editor to properly syntax-highlight the file.

Comments invited.

OK, you invited comments so I'll be the designated curmudgeon tonight... I'm not wild about uglifying our code to accommodating vim syntax highlighting limitations. Perhaps you can fix the vim syntax highlighting instead?

Quoted 11 lines
common/cmd_nvedit.c +1 -0
index 9c5d1fc..1ac91ea 100644
@@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
 		 * Ethernet Address and serial# can be set only once,
 		 * ver is readonly.
 		 */
+		if (

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

Quoted 10 lines
common/cmd_nvedit.c +2 -2
 #ifdef CONFIG_HAS_UID
 		/* Allow serial# forced overwrite with 0xdeaf4add flag */
-		if ( ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
+		    ((strcmp (name, "serial#") == 0) && (flag != 0xdeaf4add)) ||
 #else
-		if ( (strcmp (name, "serial#") == 0) ||
+		    (strcmp (name, "serial#") == 0) ||
 #endif
 		    ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)

I probably should confess I don't use syntax highlighting so that probably factors in to my lack of sympathy.

Quoted 2 lines

Best regards,
gvb

Hi Steven,

I got a good night's sleep and a cup of coffee in me and so I'm a little more mellow this morning. ;-)

Looking at the source code, its pretty ugly already. Your change doesn't make it any more ugly and it could be argued that it is slightly less ugly (your patch removes the duplication of the "if( (...)" statement). I also don't see any alternative that would make the code beautiful. :-(

Anyway, I formally withdraw my objection to this patch.

Best regards,
gvb

#3 lore raw
Quoted 59 lines · Jerry Van Baren
Quoted 43 lines · Jerry Van Baren
Quoted 9 lines · Steven A. Falco

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c
file, because it apparently does not parse C ifdef/else/endif. The following
patch does not change the behavior of the code at all, but does allow the
editor to properly syntax-highlight the file.

Comments invited.

OK, you invited comments so I'll be the designated curmudgeon tonight... I'm not wild about uglifying our code to accommodating vim syntax highlighting limitations. Perhaps you can fix the vim syntax highlighting instead?

Quoted 18 lines
common/cmd_nvedit.c +1 -1
index 9c5d1fc..1ac91ea 100644
@@ -179,11 +179,12 @@ int _do_setenv (int flag, int argc, char *argv[])
          * Ethernet Address and serial# can be set only once,
          * ver is readonly.
          */
+        if (
 #ifdef CONFIG_HAS_UID
         /* Allow serial# forced overwrite with 0xdeaf4add flag */
-        if ( ((strcmp (name, "serial#") == 0) && (flag !=
0xdeaf4add)) ||
+            ((strcmp (name, "serial#") == 0) && (flag !=
0xdeaf4add)) ||

#else

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

Quoted 5 lines
common/cmd_nvedit.c +1 -1
-        if ( (strcmp (name, "serial#") == 0) ||
+            (strcmp (name, "serial#") == 0) ||
 #endif
             ((strcmp (name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)

I probably should confess I don't use syntax highlighting so that probably factors in to my lack of sympathy.

Quoted 2 lines

Best regards,
gvb

Hi Steven,

I got a good night's sleep and a cup of coffee in me and so I'm a little more mellow this morning. ;-)

Looking at the source code, its pretty ugly already. Your change doesn't make it any more ugly and it could be argued that it is slightly less ugly (your patch removes the duplication of the "if( (...)" statement). I also don't see any alternative that would make the code beautiful. :-(

Quoted 5 lines

Anyway, I formally withdraw my objection to this patch.

Best regards,
gvb

Glad to hear it. :-)

So this is probably my first actual contribution to u-boot. Is there something more I should do to have this patch merged into an official tree, or will the appropriate person pick it up when time permits?

Steve

#4 lore raw
In reply to Jerry Van Baren
Quoted 5 lines · On Fri, 13 Jun 2008 08:14:17 -0400 Jerry Van Baren <[email protected]>…

Looking at the source code, its pretty ugly already. Your change doesn't make it any more ugly and it could be argued that it is slightly less ugly (your patch removes the duplication of the "if( (...)" statement). I also don't see any alternative that would make the code beautiful. :-(

Actually, I think the following would be slightly cleaner:

#ifdef CONFIG_HAS_UID

# define HAS_UID	(1)
#else
# define HAS_UID	(0)
#endif

/* ... */

	if (((strcmp (name, "serial#") == 0)
			&& (!HAS_UID || (flag != 0xdeaf4add)))
		|| ((strcmp (name, "ethaddr") == 0)

(I may have miscounted the parentheses though...I certainly see the need for working syntax highlighting on this code ;-)

Haavard

#5 lore raw
In reply to Steven A. Falco
Quoted 9 lines · In message <[email protected]> you wrote:

My text-editor (vim) has a bit of trouble syntax-highlighting the cmd_nvedit.c file, because it apparently does not parse C ifdef/else/endif. The following patch does not change the behavior of the code at all, but does allow the editor to properly syntax-highlight the file.

Comments invited.

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 8 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 so-called "desktop metaphor" of today's workstations  is  instead
an  "airplane-seat"  metaphor.  Anyone who has shuffled a lap full of
papers while seated between two portly passengers will recognize  the
difference -- one can see only a very few things at once.
                                                   - Fred Brooks, Jr.