All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
@ 2011-04-15  7:02 Macpaul Lin
  2011-04-15  8:00 ` Mike Frysinger
  0 siblings, 1 reply; 20+ messages in thread
From: Macpaul Lin @ 2011-04-15  7:02 UTC (permalink / raw)
  To: u-boot

Code clean up of cmd_nvedit.c by using checkpatch.pl.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Note:
  This patch has fixed most of the ERRORs and WARNINGs in cmd_nvedit.c.
  However, the remains were not clean up for avoiding broken the consistence.

 common/cmd_nvedit.c |  146 +++++++++++++++++++++++++--------------------------
 1 files changed, 72 insertions(+), 74 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 6d8512a..af621d7 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -55,15 +55,15 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if !defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
-    !defined(CONFIG_ENV_IS_IN_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
-    !defined(CONFIG_ENV_IS_IN_MMC)  && \
-    !defined(CONFIG_ENV_IS_IN_NAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
-    !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_NOWHERE)
+	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
+	!defined(CONFIG_ENV_IS_IN_MMC)  && \
+	!defined(CONFIG_ENV_IS_IN_NAND)	&& \
+	!defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
+	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
+	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
 SPI_FLASH|MG_DISK|NVRAM|MMC|NOWHERE}
 #endif
@@ -93,7 +93,7 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
  */
 static int env_id = 1;
 
-int get_env_id (void)
+int get_env_id(void)
 {
 	return env_id;
 }
@@ -116,7 +116,7 @@ static int env_print(char *name)
 		hsearch_r(e, FIND, &ep, &env_htab);
 		if (ep == NULL)
 			return 0;
-		len = printf ("%s=%s\n", ep->key, ep->data);
+		len = printf("%s=%s\n", ep->key, ep->data);
 		return len;
 	}
 
@@ -176,7 +176,7 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	name = argv[1];
 
 	if (strchr(name, '=')) {
-		printf ("## Error: illegal character '=' in variable name \"%s\"\n", name);
+		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
 		return 1;
 	}
 
@@ -189,13 +189,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	hsearch_r(e, FIND, &ep, &env_htab);
 
 	/* Check for console redirection */
-	if (strcmp(name,"stdin") == 0) {
+	if (strcmp(name, "stdin") == 0)
 		console = stdin;
-	} else if (strcmp(name,"stdout") == 0) {
+	else if (strcmp(name, "stdout") == 0)
 		console = stdout;
-	} else if (strcmp(name,"stderr") == 0) {
+	else if (strcmp(name, "stderr") == 0)
 		console = stderr;
-	}
 
 	if (console != -1) {
 		if (argc < 3) {		/* Cannot delete it! */
@@ -209,11 +208,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			return i;
 #else
 		/* Try assigning specified device */
-		if (console_assign (console, argv[2]) < 0)
+		if (console_assign(console, argv[2]) < 0)
 			return 1;
 
 #ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign (argv[2]) < 0)
+		if (serial_assign(argv[2]) < 0)
 			return 1;
 #endif
 #endif /* CONFIG_CONSOLE_MUX */
@@ -225,28 +224,28 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 */
 	if (ep) {		/* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
-		if ((strcmp (name, "serial#") == 0) ||
-		    ((strcmp (name, "ethaddr") == 0)
+		if ((strcmp(name, "serial#") == 0) ||
+		    ((strcmp(name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
-		     && (strcmp (ep->data,MK_STR(CONFIG_ETHADDR)) != 0)
+		     && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
 		    ) ) {
-			printf ("Can't overwrite \"%s\"\n", name);
+			printf("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}
 #endif
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
-		if (strcmp(name,"baudrate") == 0) {
+		if (strcmp(name, "baudrate") == 0) {
 			int baudrate = simple_strtoul(argv[2], NULL, 10);
 			int i;
-			for (i=0; i<N_BAUDRATES; ++i) {
+			for (i = 0; i < N_BAUDRATES; ++i) {
 				if (baudrate == baudrate_table[i])
 					break;
 			}
 			if (i == N_BAUDRATES) {
-				printf ("## Baudrate %d bps not supported\n",
+				printf("## Baudrate %d bps not supported\n",
 					baudrate);
 				return 1;
 			}
@@ -258,11 +257,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			gd->bd->bi_baudrate = baudrate;
 #endif
 
-			serial_setbrg ();
+			serial_setbrg();
 			udelay(50000);
 			for (;;) {
 				if (getc() == '\r')
-				      break;
+					break;
 			}
 		}
 	}
@@ -276,14 +275,15 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	/*
 	 * Insert / replace new value
 	 */
-	for (i=2,len=0; i<argc; ++i) {
+	for (i = 2, len = 0; i < argc; ++i)
 		len += strlen(argv[i]) + 1;
-	}
-	if ((value = malloc(len)) == NULL) {
+
+	value = malloc(len);
+	if (value == NULL) {
 		printf("## Can't malloc %d bytes\n", len);
 		return 1;
 	}
-	for (i=2,s=value; i<argc; ++i) {
+	for (i = 2, s = value; i < argc; ++i) {
 		char *v = argv[i];
 
 		while ((*s++ = *v++) != '\0')
@@ -308,12 +308,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 * entry in the environment is changed
 	 */
 
-	if (strcmp(name,"ipaddr") == 0) {
+	if (strcmp(name, "ipaddr") == 0) {
 		char *s = argv[2];	/* always use only one arg */
 		char *e;
 		unsigned long addr;
 		bd->bi_ip_addr = 0;
-		for (addr=0, i=0; i<4; ++i) {
+		for (addr = 0, i = 0; i < 4; ++i) {
 			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
 			addr <<= 8;
 			addr  |= (val & 0xFF);
@@ -321,20 +321,20 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		}
 		bd->bi_ip_addr = htonl(addr);
 		return 0;
-	} else if (strcmp(argv[1],"loadaddr") == 0) {
+	} else if (strcmp(argv[1], "loadaddr") == 0) {
 		load_addr = simple_strtoul(argv[2], NULL, 16);
 		return 0;
 	}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1],"bootfile") == 0) {
-		copy_filename (BootFile, argv[2], sizeof(BootFile));
+	else if (strcmp(argv[1], "bootfile") == 0) {
+		copy_filename(BootFile, argv[2], sizeof(BootFile));
 		return 0;
 	}
 #endif
 	return 0;
 }
 
-int setenv (char *varname, char *varvalue)
+int setenv(char *varname, char *varvalue)
 {
 	char * const argv[4] = { "setenv", varname, varvalue, NULL };
 	if ((varvalue == NULL) || (varvalue[0] == '\0'))
@@ -343,7 +343,7 @@ int setenv (char *varname, char *varvalue)
 		return _do_env_set(0, 3, argv);
 }
 
-int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
 		return cmd_usage(cmdtp);
@@ -355,7 +355,7 @@ int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * Prompt for environment variable
  */
 #if defined(CONFIG_CMD_ASKENV)
-int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	extern char console_buffer[CONFIG_SYS_CBSIZE];
 	char message[CONFIG_SYS_CBSIZE];
@@ -383,10 +383,10 @@ int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		break;
 
 	default:	/* env_ask envname message1 ... messagen size */
-		for (i=2,pos=0; i < argc - 1; i++) {
-			if (pos) {
+		for (i = 2, pos = 0; i < argc - 1; i++) {
+			if (pos)
 				message[pos++] = ' ';
-			}
+
 			strcpy(message+pos, argv[i]);
 			pos += strlen(argv[i]);
 		}
@@ -449,7 +449,7 @@ int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * return address of storage for that variable,
  * or NULL if not found
  */
-char *getenv (char *name)
+char *getenv(char *name)
 {
 	if (gd->flags & GD_FLG_ENV_READY) {	/* after import into hashtable */
 		ENTRY e, *ep;
@@ -460,7 +460,7 @@ char *getenv (char *name)
 		e.data = NULL;
 		hsearch_r(e, FIND, &ep, &env_htab);
 
-		return (ep ? ep->data : NULL);
+		return ep ? ep->data : NULL;
 	}
 
 	/* restricted capabilities before import */
@@ -474,23 +474,24 @@ char *getenv (char *name)
 /*
  * Look up variable from environment for restricted C runtime env.
  */
-int getenv_f (char *name, char *buf, unsigned len)
+int getenv_f(char *name, char *buf, unsigned len)
 {
 	int i, nxt;
 
-	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
+	for (i = 0; env_get_char(i) != '\0'; i = nxt+1) {
 		int val, n;
 
-		for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) {
-			if (nxt >= CONFIG_ENV_SIZE) {
-				return (-1);
-			}
+		for (nxt = i; env_get_char(nxt) != '\0'; ++nxt) {
+			if (nxt >= CONFIG_ENV_SIZE)
+				return -1;
 		}
-		if ((val=envmatch((uchar *)name, i)) < 0)
+
+		val = envmatch((uchar *)name, i);
+		if (val < 0)
 			continue;
 
 		/* found; copy out */
-		for (n=0; n<len; ++n, ++buf) {
+		for (n = 0; n < len; ++n, ++buf) {
 			if ((*buf = env_get_char(val++)) == '\0')
 				return n;
 		}
@@ -502,18 +503,18 @@ int getenv_f (char *name, char *buf, unsigned len)
 
 		return n;
 	}
-	return (-1);
+	return -1;
 }
 
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
 
-int do_env_save (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	extern char * env_name_spec;
+	extern char *env_name_spec;
 
-	printf ("Saving Environment to %s...\n", env_name_spec);
+	printf("Saving Environment to %s...\n", env_name_spec);
 
-	return (saveenv() ? 1 : 0);
+	return saveenv() ? 1 : 0;
 }
 
 U_BOOT_CMD(
@@ -533,27 +534,26 @@ U_BOOT_CMD(
  * If the names match, return the index for the value2, else NULL.
  */
 
-int envmatch (uchar *s1, int i2)
+int envmatch(uchar *s1, int i2)
 {
-
 	while (*s1 == env_get_char(i2++))
 		if (*s1++ == '=')
-			return(i2);
+			return i2;
 	if (*s1 == '\0' && env_get_char(i2-1) == '=')
-		return(i2);
-	return(-1);
+		return i2;
+	return -1;
 }
 
-static int do_env_default(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0)) {
+	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
 		return cmd_usage(cmdtp);
-	}
+
 	set_default_env("## Resetting to default environment\n");
 	return 0;
 }
 
-static int do_env_delete(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_delete(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	printf("Not implemented yet\n");
 	return 0;
@@ -639,9 +639,8 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	addr = (char *)simple_strtoul(argv[0], NULL, 16);
 
@@ -685,7 +684,7 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		envp->flags = ACTIVE_FLAG;
 #endif
 	}
-	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t,data)));
+	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t, data)));
 	setenv("filesize", buf);
 
 	return 0;
@@ -708,7 +707,7 @@ sep_err:
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
  */
-static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char	*cmd, *addr;
 	char	sep = '\n';
@@ -748,9 +747,8 @@ static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const arg
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
@@ -807,7 +805,7 @@ sep_err:
 }
 
 #if defined(CONFIG_CMD_RUN)
-extern int do_run (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
 /*
@@ -841,7 +839,7 @@ void env_reloc(void)
 }
 #endif
 
-static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	cmd_tbl_t *cp;
 
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15  7:02 [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch Macpaul Lin
@ 2011-04-15  8:00 ` Mike Frysinger
  2011-04-15  8:25   ` Macpaul Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Mike Frysinger @ 2011-04-15  8:00 UTC (permalink / raw)
  To: u-boot

On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
>  #if !defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
> -    !defined(CONFIG_ENV_IS_IN_FLASH)	&& \
> +	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \

not sure this one hunk is desirable, but the rest are fine
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110415/f4166bc6/attachment.pgp 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15  8:00 ` Mike Frysinger
@ 2011-04-15  8:25   ` Macpaul Lin
  2011-04-15  8:53     ` Mike Frysinger
  0 siblings, 1 reply; 20+ messages in thread
From: Macpaul Lin @ 2011-04-15  8:25 UTC (permalink / raw)
  To: u-boot

Hi Mike,

2011/4/15 Mike Frysinger <vapier@gentoo.org>:
> On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
>> ?#if !defined(CONFIG_ENV_IS_IN_EEPROM) ? ? ? ?&& \
>> - ? ?!defined(CONFIG_ENV_IS_IN_FLASH) && \
>> + ? ? !defined(CONFIG_ENV_IS_IN_FLASH) ? ? ? ?&& \
>
> not sure this one hunk is desirable, but the rest are fine
> -mike
>

Do you mean the replacement with space to ident?

According to the 2.6.38.1's checkpatch, it reported:
WARNING: please, no spaces at the start of a line
#90: FILE: common/cmd_bdinfo.c:61:
+    defined(CONFIG_8260) || defined(CONFIG_E500)$

We have got lots of such warnings.

I think we can use little pieces of times to do the cleanup work.
If Wolfgang has no comment to such coding style clean up,
I'll do it slowly when I have time.

Thanks.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15  8:25   ` Macpaul Lin
@ 2011-04-15  8:53     ` Mike Frysinger
  2011-04-15  8:57       ` Macpaul Lin
  2011-04-15 10:09       ` Wolfgang Denk
  0 siblings, 2 replies; 20+ messages in thread
From: Mike Frysinger @ 2011-04-15  8:53 UTC (permalink / raw)
  To: u-boot

On Friday, April 15, 2011 04:25:58 Macpaul Lin wrote:
> 2011/4/15 Mike Frysinger:
> > On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
> >>  #if !defined(CONFIG_ENV_IS_IN_EEPROM)        && \
> >> -    !defined(CONFIG_ENV_IS_IN_FLASH) && \
> >> +     !defined(CONFIG_ENV_IS_IN_FLASH)        && \
> > 
> > not sure this one hunk is desirable, but the rest are fine
> > -mike
> 
> Do you mean the replacement with space to ident?
> 
> According to the 2.6.38.1's checkpatch, it reported:
> WARNING: please, no spaces at the start of a line

this particular check has already been posted upstream as generally causing 
more harm than good.  but getting checkpatch fixed often feels glacial.

up to Wolfgang how he feels about ifdef indentation
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110415/a00ad3df/attachment.pgp 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15  8:53     ` Mike Frysinger
@ 2011-04-15  8:57       ` Macpaul Lin
  2011-04-15 10:09       ` Wolfgang Denk
  1 sibling, 0 replies; 20+ messages in thread
From: Macpaul Lin @ 2011-04-15  8:57 UTC (permalink / raw)
  To: u-boot

Hi Mike,

2011/4/15 Mike Frysinger <vapier@gentoo.org>:
>> Do you mean the replacement with space to ident?
>>
>> According to the 2.6.38.1's checkpatch, it reported:
>> WARNING: please, no spaces at the start of a line
>
> this particular check has already been posted upstream as generally causing
> more harm than good. ?but getting checkpatch fixed often feels glacial.

Thanks for your information, I didn't read that article before.
Could you provide the URL of related discussion that could be found in
the mailing list?

> up to Wolfgang how he feels about ifdef indentation
> -mike

Sure. I think we can discuss about it and then decides how to deal with it. :)
Just waiting for Wolfgang's opinion and see if the revised patch is
need to be send.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15  8:53     ` Mike Frysinger
  2011-04-15  8:57       ` Macpaul Lin
@ 2011-04-15 10:09       ` Wolfgang Denk
  2011-04-15 10:18         ` Mike Frysinger
  2011-04-16  6:22         ` Albert ARIBAUD
  1 sibling, 2 replies; 20+ messages in thread
From: Wolfgang Denk @ 2011-04-15 10:09 UTC (permalink / raw)
  To: u-boot

Dear Mike Frysinger,

In message <201104150453.21441.vapier@gentoo.org> you wrote:
>
> > > On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
> > >>  #if !defined(CONFIG_ENV_IS_IN_EEPROM)        && \
> > >> -    !defined(CONFIG_ENV_IS_IN_FLASH) && \
> > >> +     !defined(CONFIG_ENV_IS_IN_FLASH)        && \
> > > 
> > > not sure this one hunk is desirable, but the rest are fine
> > > -mike
> > 
> > Do you mean the replacement with space to ident?
> > 
> > According to the 2.6.38.1's checkpatch, it reported:
> > WARNING: please, no spaces at the start of a line
>
> this particular check has already been posted upstream as generally causing
> more harm than good.  but getting checkpatch fixed often feels glacial.
>
> up to Wolfgang how he feels about ifdef indentation

In this specific case of #ifdef indentation I feel that the original
form (which causes checkpatch warnings) is actually easier to read, so
I tend to keep it.  But I am aware that this is inconsequent as we ask
for "indentation by TAB only" everywhere else.

So I'm really torn - if it was for myself only I would not hesitate to
leave this part as is, but I realize that quite a number of users have
already complained about the unclear rules and exceptions we make with
the coding style requirements, and some actually threw in the sponge
because of that.

I'm afraid I'm not much of help here.

Best regards,

Wolfgang Denk

-- 
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
Alles Gescheite ist schon gedacht worden, man mu? nur versuchen,
es noch einmal zu denken.          -- Goethe, Maximen und Reflexionen

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15 10:09       ` Wolfgang Denk
@ 2011-04-15 10:18         ` Mike Frysinger
  2011-04-16  6:18           ` Macpaul Lin
  2011-04-16  6:22         ` Albert ARIBAUD
  1 sibling, 1 reply; 20+ messages in thread
From: Mike Frysinger @ 2011-04-15 10:18 UTC (permalink / raw)
  To: u-boot

On Friday, April 15, 2011 06:09:12 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > > > On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
> > > >>  #if !defined(CONFIG_ENV_IS_IN_EEPROM)        && \
> > > >> 
> > > >> -    !defined(CONFIG_ENV_IS_IN_FLASH) && \
> > > >> +     !defined(CONFIG_ENV_IS_IN_FLASH)        && \
> > > > 
> > > > not sure this one hunk is desirable, but the rest are fine
> > > 
> > > Do you mean the replacement with space to ident?
> > > 
> > > According to the 2.6.38.1's checkpatch, it reported:
> > > WARNING: please, no spaces at the start of a line
> > 
> > this particular check has already been posted upstream as generally
> > causing more harm than good.  but getting checkpatch fixed often feels
> > glacial.
> > 
> > up to Wolfgang how he feels about ifdef indentation
> 
> In this specific case of #ifdef indentation I feel that the original
> form (which causes checkpatch warnings) is actually easier to read, so
> I tend to keep it.  But I am aware that this is inconsequent as we ask
> for "indentation by TAB only" everywhere else.
> 
> So I'm really torn - if it was for myself only I would not hesitate to
> leave this part as is, but I realize that quite a number of users have
> already complained about the unclear rules and exceptions we make with
> the coding style requirements, and some actually threw in the sponge
> because of that.

well, this is a case where i would say a soft rule is OK -- i.e. allow both 
options and let the active maintainer of the code in question decide.  i too 
prefer the current style as i find it easier to read, but if someone 
maintaining code i never work on wants to be strict about tabs, then it's no 
sweat off my back.

so, not to dupe another thread, but i'd say "use common sense".  but that 
probably too isn't consistent/clear enough for many people.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110415/97f451b9/attachment.pgp 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15 10:18         ` Mike Frysinger
@ 2011-04-16  6:18           ` Macpaul Lin
  2011-04-16  6:35             ` Albert ARIBAUD
  0 siblings, 1 reply; 20+ messages in thread
From: Macpaul Lin @ 2011-04-16  6:18 UTC (permalink / raw)
  To: u-boot

HI all,

2011/4/15 Mike Frysinger <vapier@gentoo.org>:
>> > up to Wolfgang how he feels about ifdef indentation
>>
>> In this specific case of #ifdef indentation I feel that the original
>> form (which causes checkpatch warnings) is actually easier to read, so
>> I tend to keep it. ?But I am aware that this is inconsequent as we ask
>> for "indentation by TAB only" everywhere else.

For this specific #ifdef case, I will agree to keep the origin format,
it's exactly easier to read.
Because I'm not in the office, I can only send the fixed patch v2 until 4/18.

> well, this is a case where i would say a soft rule is OK -- i.e. allow both
> options and let the active maintainer of the code in question decide. ?i too
> prefer the current style as i find it easier to read, but if someone
> maintaining code i never work on wants to be strict about tabs, then it's no
> sweat off my back.
>
> so, not to dupe another thread, but i'd say "use common sense". ?but that
> probably too isn't consistent/clear enough for many people.
> -mike

Yes, use common sense is really too lose for many people.

Checkpatch is really help for people to maintain the consistence for
coding style and also avoid the coding method which might lead logic
failure.
However, the drawback is that we still have effort to think about if
the warning should be correct up according to every case that has been
 reported by checkpatch.

For example, 80 charecters is the most often case. Sometimes the code
is really easier to be read in the single one line. Sometimes the
complicated code that should be execute in one line is really hard to
be modified into short format to avoid exceeding the 80 charecters
lenght rule. Although most of time people can decide which format is
better for human reading than checkpatch's parsing result.
We still have effort to inform the patch commiters to modified the
code and discuss with it.

Also, the coding style clean up for the old code already exist in git
repository will help the new contributers and reduce the disscusion
effort passively.
That's why I think we can do clean up for old code slowly when someone
have time.

Maybe list a exception form and some soft rules on the web page is
sometimes help to the contributers just new to the u-boot project.

I have an idea to recruit some colledgue students locally as
volunteers to clean up the old code according to "checkpatch". They
just do code clean up. By doing this activity will also make them be
familiar with the open source projects. At the same time they can also
be familiar with git CVS. Of course a guideline for such code clean up
activity must be work out first.

Don't know what do you think of it?

Thanks.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-15 10:09       ` Wolfgang Denk
  2011-04-15 10:18         ` Mike Frysinger
@ 2011-04-16  6:22         ` Albert ARIBAUD
  2011-04-16  7:07           ` Graeme Russ
  1 sibling, 1 reply; 20+ messages in thread
From: Albert ARIBAUD @ 2011-04-16  6:22 UTC (permalink / raw)
  To: u-boot

Hi all,

Le 15/04/2011 12:09, Wolfgang Denk a ?crit :
> Dear Mike Frysinger,
>
> In message<201104150453.21441.vapier@gentoo.org>  you wrote:
>>
>>>> On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
>>>>>   #if !defined(CONFIG_ENV_IS_IN_EEPROM)&&  \
>>>>> -    !defined(CONFIG_ENV_IS_IN_FLASH)&&  \
>>>>> +     !defined(CONFIG_ENV_IS_IN_FLASH)&&  \
>>>>
>>>> not sure this one hunk is desirable, but the rest are fine
>>>> -mike
>>>
>>> Do you mean the replacement with space to ident?
>>>
>>> According to the 2.6.38.1's checkpatch, it reported:
>>> WARNING: please, no spaces at the start of a line
>>
>> this particular check has already been posted upstream as generally causing
>> more harm than good.  but getting checkpatch fixed often feels glacial.

Been there too, once. :(

>> up to Wolfgang how he feels about ifdef indentation
>
> In this specific case of #ifdef indentation I feel that the original
> form (which causes checkpatch warnings) is actually easier to read, so
> I tend to keep it.  But I am aware that this is inconsequent as we ask
> for "indentation by TAB only" everywhere else.
>
> So I'm really torn - if it was for myself only I would not hesitate to
> leave this part as is, but I realize that quite a number of users have
> already complained about the unclear rules and exceptions we make with
> the coding style requirements, and some actually threw in the sponge
> because of that.
>
> I'm afraid I'm not much of help here.

If the goal is to have the !defined aligned, then maybe a solution here 
would be toput a tab rather than a space after the #if?

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-16  6:18           ` Macpaul Lin
@ 2011-04-16  6:35             ` Albert ARIBAUD
  0 siblings, 0 replies; 20+ messages in thread
From: Albert ARIBAUD @ 2011-04-16  6:35 UTC (permalink / raw)
  To: u-boot

Le 16/04/2011 08:18, Macpaul Lin a ?crit :
> HI all,
>
> 2011/4/15 Mike Frysinger<vapier@gentoo.org>:
>>>> up to Wolfgang how he feels about ifdef indentation
>>>
>>> In this specific case of #ifdef indentation I feel that the original
>>> form (which causes checkpatch warnings) is actually easier to read, so
>>> I tend to keep it.  But I am aware that this is inconsequent as we ask
>>> for "indentation by TAB only" everywhere else.
>
> For this specific #ifdef case, I will agree to keep the origin format,
> it's exactly easier to read.
> Because I'm not in the office, I can only send the fixed patch v2 until 4/18.
>
>> well, this is a case where i would say a soft rule is OK -- i.e. allow both
>> options and let the active maintainer of the code in question decide.  i too
>> prefer the current style as i find it easier to read, but if someone
>> maintaining code i never work on wants to be strict about tabs, then it's no
>> sweat off my back.
>>
>> so, not to dupe another thread, but i'd say "use common sense".  but that
>> probably too isn't consistent/clear enough for many people.
>> -mike
>
> Yes, use common sense is really too lose for many people.
>
> Checkpatch is really help for people to maintain the consistence for
> coding style and also avoid the coding method which might lead logic
> failure.
> However, the drawback is that we still have effort to think about if
> the warning should be correct up according to every case that has been
>   reported by checkpatch.
>
> For example, 80 charecters is the most often case. Sometimes the code
> is really easier to be read in the single one line. Sometimes the
> complicated code that should be execute in one line is really hard to
> be modified into short format to avoid exceeding the 80 charecters
> lenght rule. Although most of time people can decide which format is
> better for human reading than checkpatch's parsing result.
> We still have effort to inform the patch commiters to modified the
> code and discuss with it.
>
> Also, the coding style clean up for the old code already exist in git
> repository will help the new contributers and reduce the disscusion
> effort passively.
> That's why I think we can do clean up for old code slowly when someone
> have time.
>
> Maybe list a exception form and some soft rules on the web page is
> sometimes help to the contributers just new to the u-boot project.

Ack to the whole of this. Until there is such a list, the casual patch 
committers will either not apply checkpatch, or apply a zero-warning 
strategy.

> I have an idea to recruit some colledgue students locally as
> volunteers to clean up the old code according to "checkpatch". They
> just do code clean up. By doing this activity will also make them be
> familiar with the open source projects. At the same time they can also
> be familiar with git CVS. Of course a guideline for such code clean up
> activity must be work out first.
>
> Don't know what do you think of it?

Maybe your students could successfully get some U-boot-friendly patches 
applied to checkpatch? I tried some time ago for an option to control 
the line length, but gave up because there seemed to be no reaction -- 
see <https://patchwork.kernel.org/patch/149941/>. Having someone who's 
job description actually includes "nagging the checkpatch maintainer 
until the frigging patches are accepted" could help.

Maybe a better option than adding ad hoc options would be a single patch 
to make checkpatch.pl load a custom set of checking rules that could 
supersede the original ones. The set could be contained in the current 
directory as ./.checkpatch.rules (and maybe also look in $(HOME) if not 
found in $(PWD). Not too sure this is technically feasible, though.

> Thanks.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-16  6:22         ` Albert ARIBAUD
@ 2011-04-16  7:07           ` Graeme Russ
  2011-04-16 11:17             ` Macpaul Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Graeme Russ @ 2011-04-16  7:07 UTC (permalink / raw)
  To: u-boot

On 16/04/11 16:22, Albert ARIBAUD wrote:
> Hi all,
> 
> Le 15/04/2011 12:09, Wolfgang Denk a ?crit :
>> Dear Mike Frysinger,
>>
>> In message<201104150453.21441.vapier@gentoo.org>  you wrote:
>>>
>>>>> On Friday, April 15, 2011 03:02:58 Macpaul Lin wrote:
>>>>>>   #if !defined(CONFIG_ENV_IS_IN_EEPROM)&&  \
>>>>>> -    !defined(CONFIG_ENV_IS_IN_FLASH)&&  \
>>>>>> +     !defined(CONFIG_ENV_IS_IN_FLASH)&&  \
>>>>>

[snip]

> 
> If the goal is to have the !defined aligned, then maybe a solution here 
> would be toput a tab rather than a space after the #if?
> 

I agree - Aligned looks cleaner and easier to read (especially if the logic
is multi-levelled) and shutting up checkpatch by using a tab rather than a
space is an easy solution

But, fwiw, once the patch is applied and everything is nicely aligned and
reads well, who's going to care that checkpatch complained?

Regards,

Graeme

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-16  7:07           ` Graeme Russ
@ 2011-04-16 11:17             ` Macpaul Lin
  2011-04-16 12:55               ` Albert ARIBAUD
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Macpaul Lin @ 2011-04-16 11:17 UTC (permalink / raw)
  To: u-boot

Hi all,

2011/4/16 Graeme Russ <graeme.russ@gmail.com>:
> On 16/04/11 16:22, Albert ARIBAUD wrote:
>>
>> If the goal is to have the !defined aligned, then maybe a solution here
>> would be toput a tab rather than a space after the #if?
>>

Do you guys means
#if"\t"defined (XXXXXX) ||
"\t"defined (XXXXXX)
instead of
#if defined (XXXXXX) ||
"space*4" defined (XXXXXX)
?

If so, I'll fix the patch and then send v2 on next Monday and see if
everybody feel this will be better.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch
  2011-04-16 11:17             ` Macpaul Lin
@ 2011-04-16 12:55               ` Albert ARIBAUD
  2011-04-25  8:55               ` [U-Boot] [PATCH v2] " Macpaul Lin
  2011-04-27  2:16               ` [U-Boot] [PATCH v3] " Macpaul Lin
  2 siblings, 0 replies; 20+ messages in thread
From: Albert ARIBAUD @ 2011-04-16 12:55 UTC (permalink / raw)
  To: u-boot

Hi Macpaul Lin,

Le 16/04/2011 13:17, Macpaul Lin a ?crit :
> Hi all,
>
> 2011/4/16 Graeme Russ<graeme.russ@gmail.com>:
>> On 16/04/11 16:22, Albert ARIBAUD wrote:
>>>
>>> If the goal is to have the !defined aligned, then maybe a solution here
>>> would be toput a tab rather than a space after the #if?
>>>
>
> Do you guys means
> #if"\t"defined (XXXXXX) ||
> "\t"defined (XXXXXX)
> instead of
> #if defined (XXXXXX) ||
> "space*4" defined (XXXXXX)
> ?

Yes, this is what I mean.

> If so, I'll fix the patch and then send v2 on next Monday and see if
> everybody feel this will be better.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2] cmd_nvedit.c: clean up with checkpatch
  2011-04-16 11:17             ` Macpaul Lin
  2011-04-16 12:55               ` Albert ARIBAUD
@ 2011-04-25  8:55               ` Macpaul Lin
  2011-04-27 10:11                 ` Detlev Zundel
  2011-04-27  2:16               ` [U-Boot] [PATCH v3] " Macpaul Lin
  2 siblings, 1 reply; 20+ messages in thread
From: Macpaul Lin @ 2011-04-25  8:55 UTC (permalink / raw)
  To: u-boot

Code clean up of cmd_nvedit.c by using checkpatch.pl.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>

---
Changes for v2:
  - Replace the ident characters for #ifdef from space to tab.

 common/cmd_nvedit.c |  148 +++++++++++++++++++++++++--------------------------
 1 files changed, 73 insertions(+), 75 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 6d8512a..5fb5a92 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -54,16 +54,16 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if !defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
-    !defined(CONFIG_ENV_IS_IN_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
-    !defined(CONFIG_ENV_IS_IN_MMC)  && \
-    !defined(CONFIG_ENV_IS_IN_NAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
-    !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_NOWHERE)
+#if	!defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
+	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
+	!defined(CONFIG_ENV_IS_IN_MMC)		&& \
+	!defined(CONFIG_ENV_IS_IN_NAND)		&& \
+	!defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
+	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
+	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
 SPI_FLASH|MG_DISK|NVRAM|MMC|NOWHERE}
 #endif
@@ -93,7 +93,7 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
  */
 static int env_id = 1;
 
-int get_env_id (void)
+int get_env_id(void)
 {
 	return env_id;
 }
@@ -116,7 +116,7 @@ static int env_print(char *name)
 		hsearch_r(e, FIND, &ep, &env_htab);
 		if (ep == NULL)
 			return 0;
-		len = printf ("%s=%s\n", ep->key, ep->data);
+		len = printf("%s=%s\n", ep->key, ep->data);
 		return len;
 	}
 
@@ -176,7 +176,7 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	name = argv[1];
 
 	if (strchr(name, '=')) {
-		printf ("## Error: illegal character '=' in variable name \"%s\"\n", name);
+		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
 		return 1;
 	}
 
@@ -189,13 +189,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	hsearch_r(e, FIND, &ep, &env_htab);
 
 	/* Check for console redirection */
-	if (strcmp(name,"stdin") == 0) {
+	if (strcmp(name, "stdin") == 0)
 		console = stdin;
-	} else if (strcmp(name,"stdout") == 0) {
+	else if (strcmp(name, "stdout") == 0)
 		console = stdout;
-	} else if (strcmp(name,"stderr") == 0) {
+	else if (strcmp(name, "stderr") == 0)
 		console = stderr;
-	}
 
 	if (console != -1) {
 		if (argc < 3) {		/* Cannot delete it! */
@@ -209,11 +208,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			return i;
 #else
 		/* Try assigning specified device */
-		if (console_assign (console, argv[2]) < 0)
+		if (console_assign(console, argv[2]) < 0)
 			return 1;
 
 #ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign (argv[2]) < 0)
+		if (serial_assign(argv[2]) < 0)
 			return 1;
 #endif
 #endif /* CONFIG_CONSOLE_MUX */
@@ -225,28 +224,28 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 */
 	if (ep) {		/* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
-		if ((strcmp (name, "serial#") == 0) ||
-		    ((strcmp (name, "ethaddr") == 0)
+		if ((strcmp(name, "serial#") == 0) ||
+		    ((strcmp(name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
-		     && (strcmp (ep->data,MK_STR(CONFIG_ETHADDR)) != 0)
+		     && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
 		    ) ) {
-			printf ("Can't overwrite \"%s\"\n", name);
+			printf("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}
 #endif
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
-		if (strcmp(name,"baudrate") == 0) {
+		if (strcmp(name, "baudrate") == 0) {
 			int baudrate = simple_strtoul(argv[2], NULL, 10);
 			int i;
-			for (i=0; i<N_BAUDRATES; ++i) {
+			for (i = 0; i < N_BAUDRATES; ++i) {
 				if (baudrate == baudrate_table[i])
 					break;
 			}
 			if (i == N_BAUDRATES) {
-				printf ("## Baudrate %d bps not supported\n",
+				printf("## Baudrate %d bps not supported\n",
 					baudrate);
 				return 1;
 			}
@@ -258,11 +257,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			gd->bd->bi_baudrate = baudrate;
 #endif
 
-			serial_setbrg ();
+			serial_setbrg();
 			udelay(50000);
 			for (;;) {
 				if (getc() == '\r')
-				      break;
+					break;
 			}
 		}
 	}
@@ -276,14 +275,15 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	/*
 	 * Insert / replace new value
 	 */
-	for (i=2,len=0; i<argc; ++i) {
+	for (i = 2, len = 0; i < argc; ++i)
 		len += strlen(argv[i]) + 1;
-	}
-	if ((value = malloc(len)) == NULL) {
+
+	value = malloc(len);
+	if (value == NULL) {
 		printf("## Can't malloc %d bytes\n", len);
 		return 1;
 	}
-	for (i=2,s=value; i<argc; ++i) {
+	for (i = 2, s = value; i < argc; ++i) {
 		char *v = argv[i];
 
 		while ((*s++ = *v++) != '\0')
@@ -308,12 +308,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 * entry in the environment is changed
 	 */
 
-	if (strcmp(name,"ipaddr") == 0) {
+	if (strcmp(name, "ipaddr") == 0) {
 		char *s = argv[2];	/* always use only one arg */
 		char *e;
 		unsigned long addr;
 		bd->bi_ip_addr = 0;
-		for (addr=0, i=0; i<4; ++i) {
+		for (addr = 0, i = 0; i < 4; ++i) {
 			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
 			addr <<= 8;
 			addr  |= (val & 0xFF);
@@ -321,20 +321,20 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		}
 		bd->bi_ip_addr = htonl(addr);
 		return 0;
-	} else if (strcmp(argv[1],"loadaddr") == 0) {
+	} else if (strcmp(argv[1], "loadaddr") == 0) {
 		load_addr = simple_strtoul(argv[2], NULL, 16);
 		return 0;
 	}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1],"bootfile") == 0) {
-		copy_filename (BootFile, argv[2], sizeof(BootFile));
+	else if (strcmp(argv[1], "bootfile") == 0) {
+		copy_filename(BootFile, argv[2], sizeof(BootFile));
 		return 0;
 	}
 #endif
 	return 0;
 }
 
-int setenv (char *varname, char *varvalue)
+int setenv(char *varname, char *varvalue)
 {
 	char * const argv[4] = { "setenv", varname, varvalue, NULL };
 	if ((varvalue == NULL) || (varvalue[0] == '\0'))
@@ -343,7 +343,7 @@ int setenv (char *varname, char *varvalue)
 		return _do_env_set(0, 3, argv);
 }
 
-int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
 		return cmd_usage(cmdtp);
@@ -355,7 +355,7 @@ int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * Prompt for environment variable
  */
 #if defined(CONFIG_CMD_ASKENV)
-int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	extern char console_buffer[CONFIG_SYS_CBSIZE];
 	char message[CONFIG_SYS_CBSIZE];
@@ -383,10 +383,10 @@ int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		break;
 
 	default:	/* env_ask envname message1 ... messagen size */
-		for (i=2,pos=0; i < argc - 1; i++) {
-			if (pos) {
+		for (i = 2, pos = 0; i < argc - 1; i++) {
+			if (pos)
 				message[pos++] = ' ';
-			}
+
 			strcpy(message+pos, argv[i]);
 			pos += strlen(argv[i]);
 		}
@@ -449,7 +449,7 @@ int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * return address of storage for that variable,
  * or NULL if not found
  */
-char *getenv (char *name)
+char *getenv(char *name)
 {
 	if (gd->flags & GD_FLG_ENV_READY) {	/* after import into hashtable */
 		ENTRY e, *ep;
@@ -460,7 +460,7 @@ char *getenv (char *name)
 		e.data = NULL;
 		hsearch_r(e, FIND, &ep, &env_htab);
 
-		return (ep ? ep->data : NULL);
+		return ep ? ep->data : NULL;
 	}
 
 	/* restricted capabilities before import */
@@ -474,23 +474,24 @@ char *getenv (char *name)
 /*
  * Look up variable from environment for restricted C runtime env.
  */
-int getenv_f (char *name, char *buf, unsigned len)
+int getenv_f(char *name, char *buf, unsigned len)
 {
 	int i, nxt;
 
-	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
+	for (i = 0; env_get_char(i) != '\0'; i = nxt+1) {
 		int val, n;
 
-		for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) {
-			if (nxt >= CONFIG_ENV_SIZE) {
-				return (-1);
-			}
+		for (nxt = i; env_get_char(nxt) != '\0'; ++nxt) {
+			if (nxt >= CONFIG_ENV_SIZE)
+				return -1;
 		}
-		if ((val=envmatch((uchar *)name, i)) < 0)
+
+		val = envmatch((uchar *)name, i);
+		if (val < 0)
 			continue;
 
 		/* found; copy out */
-		for (n=0; n<len; ++n, ++buf) {
+		for (n = 0; n < len; ++n, ++buf) {
 			if ((*buf = env_get_char(val++)) == '\0')
 				return n;
 		}
@@ -502,18 +503,18 @@ int getenv_f (char *name, char *buf, unsigned len)
 
 		return n;
 	}
-	return (-1);
+	return -1;
 }
 
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
 
-int do_env_save (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	extern char * env_name_spec;
+	extern char *env_name_spec;
 
-	printf ("Saving Environment to %s...\n", env_name_spec);
+	printf("Saving Environment to %s...\n", env_name_spec);
 
-	return (saveenv() ? 1 : 0);
+	return saveenv() ? 1 : 0;
 }
 
 U_BOOT_CMD(
@@ -533,27 +534,26 @@ U_BOOT_CMD(
  * If the names match, return the index for the value2, else NULL.
  */
 
-int envmatch (uchar *s1, int i2)
+int envmatch(uchar *s1, int i2)
 {
-
 	while (*s1 == env_get_char(i2++))
 		if (*s1++ == '=')
-			return(i2);
+			return i2;
 	if (*s1 == '\0' && env_get_char(i2-1) == '=')
-		return(i2);
-	return(-1);
+		return i2;
+	return -1;
 }
 
-static int do_env_default(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0)) {
+	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
 		return cmd_usage(cmdtp);
-	}
+
 	set_default_env("## Resetting to default environment\n");
 	return 0;
 }
 
-static int do_env_delete(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_delete(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	printf("Not implemented yet\n");
 	return 0;
@@ -639,9 +639,8 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	addr = (char *)simple_strtoul(argv[0], NULL, 16);
 
@@ -685,7 +684,7 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		envp->flags = ACTIVE_FLAG;
 #endif
 	}
-	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t,data)));
+	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t, data)));
 	setenv("filesize", buf);
 
 	return 0;
@@ -708,7 +707,7 @@ sep_err:
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
  */
-static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char	*cmd, *addr;
 	char	sep = '\n';
@@ -748,9 +747,8 @@ static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const arg
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
@@ -807,7 +805,7 @@ sep_err:
 #endif
 
 #if defined(CONFIG_CMD_RUN)
-extern int do_run (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
 /*
@@ -841,7 +839,7 @@ void env_reloc(void)
 }
 #endif
 
-static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	cmd_tbl_t *cp;
 
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3] cmd_nvedit.c: clean up with checkpatch
  2011-04-16 11:17             ` Macpaul Lin
  2011-04-16 12:55               ` Albert ARIBAUD
  2011-04-25  8:55               ` [U-Boot] [PATCH v2] " Macpaul Lin
@ 2011-04-27  2:16               ` Macpaul Lin
  2011-04-27 14:44                 ` Detlev Zundel
  2011-04-30 20:27                 ` Wolfgang Denk
  2 siblings, 2 replies; 20+ messages in thread
From: Macpaul Lin @ 2011-04-27  2:16 UTC (permalink / raw)
  To: u-boot

Code clean up of cmd_nvedit.c by using checkpatch.pl.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>
---
Changes for v2:
  - Replace the ident characters for #ifdef from space to tab.
Changes for v3:
  - fix for commit 31e41398, "Fix typo in #error: IS_IN_NOWHERE vs. IS_NOWHERE"

 common/cmd_nvedit.c |  148 +++++++++++++++++++++++++--------------------------
 1 files changed, 73 insertions(+), 75 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index df0e6db..c45af56 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -54,16 +54,16 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if !defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
-    !defined(CONFIG_ENV_IS_IN_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
-    !defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
-    !defined(CONFIG_ENV_IS_IN_MMC)  && \
-    !defined(CONFIG_ENV_IS_IN_NAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
-    !defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
-    !defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
-    !defined(CONFIG_ENV_IS_NOWHERE)
+#if	!defined(CONFIG_ENV_IS_IN_EEPROM)	&& \
+	!defined(CONFIG_ENV_IS_IN_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_DATAFLASH)	&& \
+	!defined(CONFIG_ENV_IS_IN_MG_DISK)	&& \
+	!defined(CONFIG_ENV_IS_IN_MMC)		&& \
+	!defined(CONFIG_ENV_IS_IN_NAND)		&& \
+	!defined(CONFIG_ENV_IS_IN_NVRAM)	&& \
+	!defined(CONFIG_ENV_IS_IN_ONENAND)	&& \
+	!defined(CONFIG_ENV_IS_IN_SPI_FLASH)	&& \
+	!defined(CONFIG_ENV_IS_NOWHERE)
 # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\
 SPI_FLASH|MG_DISK|NVRAM|MMC} or CONFIG_ENV_IS_NOWHERE
 #endif
@@ -93,7 +93,7 @@ static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
  */
 static int env_id = 1;
 
-int get_env_id (void)
+int get_env_id(void)
 {
 	return env_id;
 }
@@ -116,7 +116,7 @@ static int env_print(char *name)
 		hsearch_r(e, FIND, &ep, &env_htab);
 		if (ep == NULL)
 			return 0;
-		len = printf ("%s=%s\n", ep->key, ep->data);
+		len = printf("%s=%s\n", ep->key, ep->data);
 		return len;
 	}
 
@@ -176,7 +176,7 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	name = argv[1];
 
 	if (strchr(name, '=')) {
-		printf ("## Error: illegal character '=' in variable name \"%s\"\n", name);
+		printf("## Error: illegal character '=' in variable name \"%s\"\n", name);
 		return 1;
 	}
 
@@ -189,13 +189,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	hsearch_r(e, FIND, &ep, &env_htab);
 
 	/* Check for console redirection */
-	if (strcmp(name,"stdin") == 0) {
+	if (strcmp(name, "stdin") == 0)
 		console = stdin;
-	} else if (strcmp(name,"stdout") == 0) {
+	else if (strcmp(name, "stdout") == 0)
 		console = stdout;
-	} else if (strcmp(name,"stderr") == 0) {
+	else if (strcmp(name, "stderr") == 0)
 		console = stderr;
-	}
 
 	if (console != -1) {
 		if (argc < 3) {		/* Cannot delete it! */
@@ -209,11 +208,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			return i;
 #else
 		/* Try assigning specified device */
-		if (console_assign (console, argv[2]) < 0)
+		if (console_assign(console, argv[2]) < 0)
 			return 1;
 
 #ifdef CONFIG_SERIAL_MULTI
-		if (serial_assign (argv[2]) < 0)
+		if (serial_assign(argv[2]) < 0)
 			return 1;
 #endif
 #endif /* CONFIG_CONSOLE_MUX */
@@ -225,28 +224,28 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 */
 	if (ep) {		/* variable exists */
 #ifndef CONFIG_ENV_OVERWRITE
-		if ((strcmp (name, "serial#") == 0) ||
-		    ((strcmp (name, "ethaddr") == 0)
+		if ((strcmp(name, "serial#") == 0) ||
+		    ((strcmp(name, "ethaddr") == 0)
 #if defined(CONFIG_OVERWRITE_ETHADDR_ONCE) && defined(CONFIG_ETHADDR)
-		     && (strcmp (ep->data,MK_STR(CONFIG_ETHADDR)) != 0)
+		     && (strcmp(ep->data, MK_STR(CONFIG_ETHADDR)) != 0)
 #endif	/* CONFIG_OVERWRITE_ETHADDR_ONCE && CONFIG_ETHADDR */
 		    ) ) {
-			printf ("Can't overwrite \"%s\"\n", name);
+			printf("Can't overwrite \"%s\"\n", name);
 			return 1;
 		}
 #endif
 		/*
 		 * Switch to new baudrate if new baudrate is supported
 		 */
-		if (strcmp(name,"baudrate") == 0) {
+		if (strcmp(name, "baudrate") == 0) {
 			int baudrate = simple_strtoul(argv[2], NULL, 10);
 			int i;
-			for (i=0; i<N_BAUDRATES; ++i) {
+			for (i = 0; i < N_BAUDRATES; ++i) {
 				if (baudrate == baudrate_table[i])
 					break;
 			}
 			if (i == N_BAUDRATES) {
-				printf ("## Baudrate %d bps not supported\n",
+				printf("## Baudrate %d bps not supported\n",
 					baudrate);
 				return 1;
 			}
@@ -258,11 +257,11 @@ int _do_env_set (int flag, int argc, char * const argv[])
 			gd->bd->bi_baudrate = baudrate;
 #endif
 
-			serial_setbrg ();
+			serial_setbrg();
 			udelay(50000);
 			for (;;) {
 				if (getc() == '\r')
-				      break;
+					break;
 			}
 		}
 	}
@@ -276,14 +275,15 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	/*
 	 * Insert / replace new value
 	 */
-	for (i=2,len=0; i<argc; ++i) {
+	for (i = 2, len = 0; i < argc; ++i)
 		len += strlen(argv[i]) + 1;
-	}
-	if ((value = malloc(len)) == NULL) {
+
+	value = malloc(len);
+	if (value == NULL) {
 		printf("## Can't malloc %d bytes\n", len);
 		return 1;
 	}
-	for (i=2,s=value; i<argc; ++i) {
+	for (i = 2, s = value; i < argc; ++i) {
 		char *v = argv[i];
 
 		while ((*s++ = *v++) != '\0')
@@ -308,12 +308,12 @@ int _do_env_set (int flag, int argc, char * const argv[])
 	 * entry in the environment is changed
 	 */
 
-	if (strcmp(name,"ipaddr") == 0) {
+	if (strcmp(name, "ipaddr") == 0) {
 		char *s = argv[2];	/* always use only one arg */
 		char *e;
 		unsigned long addr;
 		bd->bi_ip_addr = 0;
-		for (addr=0, i=0; i<4; ++i) {
+		for (addr = 0, i = 0; i < 4; ++i) {
 			ulong val = s ? simple_strtoul(s, &e, 10) : 0;
 			addr <<= 8;
 			addr  |= (val & 0xFF);
@@ -321,20 +321,20 @@ int _do_env_set (int flag, int argc, char * const argv[])
 		}
 		bd->bi_ip_addr = htonl(addr);
 		return 0;
-	} else if (strcmp(argv[1],"loadaddr") == 0) {
+	} else if (strcmp(argv[1], "loadaddr") == 0) {
 		load_addr = simple_strtoul(argv[2], NULL, 16);
 		return 0;
 	}
 #if defined(CONFIG_CMD_NET)
-	else if (strcmp(argv[1],"bootfile") == 0) {
-		copy_filename (BootFile, argv[2], sizeof(BootFile));
+	else if (strcmp(argv[1], "bootfile") == 0) {
+		copy_filename(BootFile, argv[2], sizeof(BootFile));
 		return 0;
 	}
 #endif
 	return 0;
 }
 
-int setenv (char *varname, char *varvalue)
+int setenv(char *varname, char *varvalue)
 {
 	char * const argv[4] = { "setenv", varname, varvalue, NULL };
 	if ((varvalue == NULL) || (varvalue[0] == '\0'))
@@ -343,7 +343,7 @@ int setenv (char *varname, char *varvalue)
 		return _do_env_set(0, 3, argv);
 }
 
-int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
 		return cmd_usage(cmdtp);
@@ -355,7 +355,7 @@ int do_env_set (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * Prompt for environment variable
  */
 #if defined(CONFIG_CMD_ASKENV)
-int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	extern char console_buffer[CONFIG_SYS_CBSIZE];
 	char message[CONFIG_SYS_CBSIZE];
@@ -383,10 +383,10 @@ int do_env_ask ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		break;
 
 	default:	/* env_ask envname message1 ... messagen size */
-		for (i=2,pos=0; i < argc - 1; i++) {
-			if (pos) {
+		for (i = 2, pos = 0; i < argc - 1; i++) {
+			if (pos)
 				message[pos++] = ' ';
-			}
+
 			strcpy(message+pos, argv[i]);
 			pos += strlen(argv[i]);
 		}
@@ -449,7 +449,7 @@ int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  * return address of storage for that variable,
  * or NULL if not found
  */
-char *getenv (char *name)
+char *getenv(char *name)
 {
 	if (gd->flags & GD_FLG_ENV_READY) {	/* after import into hashtable */
 		ENTRY e, *ep;
@@ -460,7 +460,7 @@ char *getenv (char *name)
 		e.data = NULL;
 		hsearch_r(e, FIND, &ep, &env_htab);
 
-		return (ep ? ep->data : NULL);
+		return ep ? ep->data : NULL;
 	}
 
 	/* restricted capabilities before import */
@@ -474,23 +474,24 @@ char *getenv (char *name)
 /*
  * Look up variable from environment for restricted C runtime env.
  */
-int getenv_f (char *name, char *buf, unsigned len)
+int getenv_f(char *name, char *buf, unsigned len)
 {
 	int i, nxt;
 
-	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
+	for (i = 0; env_get_char(i) != '\0'; i = nxt+1) {
 		int val, n;
 
-		for (nxt=i; env_get_char(nxt) != '\0'; ++nxt) {
-			if (nxt >= CONFIG_ENV_SIZE) {
-				return (-1);
-			}
+		for (nxt = i; env_get_char(nxt) != '\0'; ++nxt) {
+			if (nxt >= CONFIG_ENV_SIZE)
+				return -1;
 		}
-		if ((val=envmatch((uchar *)name, i)) < 0)
+
+		val = envmatch((uchar *)name, i);
+		if (val < 0)
 			continue;
 
 		/* found; copy out */
-		for (n=0; n<len; ++n, ++buf) {
+		for (n = 0; n < len; ++n, ++buf) {
 			if ((*buf = env_get_char(val++)) == '\0')
 				return n;
 		}
@@ -502,18 +503,18 @@ int getenv_f (char *name, char *buf, unsigned len)
 
 		return n;
 	}
-	return (-1);
+	return -1;
 }
 
 #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
 
-int do_env_save (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	extern char * env_name_spec;
+	extern char *env_name_spec;
 
-	printf ("Saving Environment to %s...\n", env_name_spec);
+	printf("Saving Environment to %s...\n", env_name_spec);
 
-	return (saveenv() ? 1 : 0);
+	return saveenv() ? 1 : 0;
 }
 
 U_BOOT_CMD(
@@ -533,27 +534,26 @@ U_BOOT_CMD(
  * If the names match, return the index for the value2, else NULL.
  */
 
-int envmatch (uchar *s1, int i2)
+int envmatch(uchar *s1, int i2)
 {
-
 	while (*s1 == env_get_char(i2++))
 		if (*s1++ == '=')
-			return(i2);
+			return i2;
 	if (*s1 == '\0' && env_get_char(i2-1) == '=')
-		return(i2);
-	return(-1);
+		return i2;
+	return -1;
 }
 
-static int do_env_default(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_default(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if ((argc != 2) || (strcmp(argv[1], "-f") != 0)) {
+	if ((argc != 2) || (strcmp(argv[1], "-f") != 0))
 		return cmd_usage(cmdtp);
-	}
+
 	set_default_env("## Resetting to default environment\n");
 	return 0;
 }
 
-static int do_env_delete(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_delete(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	printf("Not implemented yet\n");
 	return 0;
@@ -640,9 +640,8 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	addr = (char *)simple_strtoul(argv[0], NULL, 16);
 
@@ -686,7 +685,7 @@ static int do_env_export(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
 		envp->flags = ACTIVE_FLAG;
 #endif
 	}
-	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t,data)));
+	sprintf(buf, "%zX", (size_t)(len + offsetof(env_t, data)));
 	setenv("filesize", buf);
 
 	return 0;
@@ -711,7 +710,7 @@ sep_err:
  *	size:	length of input data; if missing, proper '\0'
  *		termination is mandatory
  */
-static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
+static int do_env_import(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	char	*cmd, *addr;
 	char	sep = '\n';
@@ -751,9 +750,8 @@ static int do_env_import(cmd_tbl_t * cmdtp, int flag, int argc, char * const arg
 		}
 	}
 
-	if (argc < 1) {
+	if (argc < 1)
 		return cmd_usage(cmdtp);
-	}
 
 	if (!fmt)
 		printf("## Warning: defaulting to text format\n");
@@ -811,7 +809,7 @@ sep_err:
 #endif
 
 #if defined(CONFIG_CMD_RUN)
-extern int do_run (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+extern int do_run(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
 
 /*
@@ -849,7 +847,7 @@ void env_reloc(void)
 }
 #endif
 
-static int do_env (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+static int do_env(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	cmd_tbl_t *cp;
 
-- 
1.7.3.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2] cmd_nvedit.c: clean up with checkpatch
  2011-04-25  8:55               ` [U-Boot] [PATCH v2] " Macpaul Lin
@ 2011-04-27 10:11                 ` Detlev Zundel
  2011-04-27 10:53                   ` Macpaul Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Detlev Zundel @ 2011-04-27 10:11 UTC (permalink / raw)
  To: u-boot

Hi,

> Code clean up of cmd_nvedit.c by using checkpatch.pl.

Thanks for taking the initiative to do some cleanup!

> Signed-off-by: Macpaul Lin <macpaul@andestech.com>

Still, the patch fails for me:

  Applying: cmd_nvedit.c: clean up with checkpatch
  Using index info to reconstruct a base tree...
  error: patch failed: common/cmd_nvedit.c:807
  error: common/cmd_nvedit.c: patch does not apply
  Did you hand edit your patch?
  It does not apply to blobs recorded in its index.
  Cannot fall back to three-way merge.
  Patch failed at 0001 cmd_nvedit.c: clean up with checkpatch

Cheers
  Detlev

-- 
Computer scientists do it depth-first.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v2] cmd_nvedit.c: clean up with checkpatch
  2011-04-27 10:11                 ` Detlev Zundel
@ 2011-04-27 10:53                   ` Macpaul Lin
  0 siblings, 0 replies; 20+ messages in thread
From: Macpaul Lin @ 2011-04-27 10:53 UTC (permalink / raw)
  To: u-boot

Hi Detlev,

2011/4/27 Detlev Zundel <dzu@denx.de>:
> Hi,
>
>> Code clean up of cmd_nvedit.c by using checkpatch.pl.
>
> Thanks for taking the initiative to do some cleanup!
>
>> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
>
> Still, the patch fails for me:
>
> ?Applying: cmd_nvedit.c: clean up with checkpatch
> ?Using index info to reconstruct a base tree...
> ?error: patch failed: common/cmd_nvedit.c:807
> ?error: common/cmd_nvedit.c: patch does not apply
> ?Did you hand edit your patch?
> ?It does not apply to blobs recorded in its index.
> ?Cannot fall back to three-way merge.
> ?Patch failed at 0001 cmd_nvedit.c: clean up with checkpatch

Sorry when I replied your mail was doing other things busy so I didn't
catch up your reply.
I didn't hand edit my patch.
Are you applying the Patch v3?
Could you provide the git hash (commit number) to me.
The 3 commits before this patch v3 is

a01ebd: mpc52xx, digsy_mtc
71d19f30: rtc, rv3029
31e4139: Fix typo in #error

Thanks.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3] cmd_nvedit.c: clean up with checkpatch
  2011-04-27  2:16               ` [U-Boot] [PATCH v3] " Macpaul Lin
@ 2011-04-27 14:44                 ` Detlev Zundel
  2011-04-27 14:49                   ` Macpaul Lin
  2011-04-30 20:27                 ` Wolfgang Denk
  1 sibling, 1 reply; 20+ messages in thread
From: Detlev Zundel @ 2011-04-27 14:44 UTC (permalink / raw)
  To: u-boot

Hi,

> Code clean up of cmd_nvedit.c by using checkpatch.pl.
>
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>

Visual inspection shows only cosmetic changes, so the patch looks good.

For the next time, we should follow the (newly added) clause in our
wiki[1] and have a ommit text like "cosmetic: code cleanup of
cmd_nvedit.c using checkpatch.pl".

As this is very new, I won't insists, so

Acked-by: Detlev Zundel <dzu@denx.de>

Thanks!
  Detlev

-- 
Q: Does that mean we also shouldn't be using IRQF_SAMPLE_RANDOM on
   interrupt [sources]?
A: Yes. The flag needs to be taken out and shot.
                            -- Matt Mackall <1239116251.14392.133.camel@calx>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3] cmd_nvedit.c: clean up with checkpatch
  2011-04-27 14:44                 ` Detlev Zundel
@ 2011-04-27 14:49                   ` Macpaul Lin
  0 siblings, 0 replies; 20+ messages in thread
From: Macpaul Lin @ 2011-04-27 14:49 UTC (permalink / raw)
  To: u-boot

Hi Detlve,

2011/4/27 Detlev Zundel <dzu@denx.de>:
> Hi,
>
> For the next time, we should follow the (newly added) clause in our
> wiki[1] and have a ommit text like "cosmetic: code cleanup of
> cmd_nvedit.c using checkpatch.pl".

Thanks for your note when we have sometimes didn't follow the new
discuccssed things because other reasons. :)

> As this is very new, I won't insists, so
>
> Acked-by: Detlev Zundel <dzu@denx.de>
>

Thanks a lot. I'll remember the new rules. Have a nice week-end.

-- 
Best regards,
Macpaul Lin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3] cmd_nvedit.c: clean up with checkpatch
  2011-04-27  2:16               ` [U-Boot] [PATCH v3] " Macpaul Lin
  2011-04-27 14:44                 ` Detlev Zundel
@ 2011-04-30 20:27                 ` Wolfgang Denk
  1 sibling, 0 replies; 20+ messages in thread
From: Wolfgang Denk @ 2011-04-30 20:27 UTC (permalink / raw)
  To: u-boot

Dear Macpaul Lin,

In message <1303870605-31401-1-git-send-email-macpaul@andestech.com> you wrote:
> Code clean up of cmd_nvedit.c by using checkpatch.pl.
> 
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> ---
> Changes for v2:
>   - Replace the ident characters for #ifdef from space to tab.
> Changes for v3:
>   - fix for commit 31e41398, "Fix typo in #error: IS_IN_NOWHERE vs. IS_NOWHERE"
> 
>  common/cmd_nvedit.c |  148 +++++++++++++++++++++++++--------------------------
>  1 files changed, 73 insertions(+), 75 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
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 took him several minutes to understand any new idea  put  to  him,
and  this is a very valuable trait in a leader, because anything any-
one is still trying to explain to you after two minutes  is  probably
important  and anything they give up after a mere minute or so is al-
most certainly something they shouldn't have been bothering you  with
in the first place.                   - Terry Pratchett, _Reaper Man_

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-04-30 20:27 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-15  7:02 [U-Boot] [PATCH] cmd_nvedit.c: clean up with checkpatch Macpaul Lin
2011-04-15  8:00 ` Mike Frysinger
2011-04-15  8:25   ` Macpaul Lin
2011-04-15  8:53     ` Mike Frysinger
2011-04-15  8:57       ` Macpaul Lin
2011-04-15 10:09       ` Wolfgang Denk
2011-04-15 10:18         ` Mike Frysinger
2011-04-16  6:18           ` Macpaul Lin
2011-04-16  6:35             ` Albert ARIBAUD
2011-04-16  6:22         ` Albert ARIBAUD
2011-04-16  7:07           ` Graeme Russ
2011-04-16 11:17             ` Macpaul Lin
2011-04-16 12:55               ` Albert ARIBAUD
2011-04-25  8:55               ` [U-Boot] [PATCH v2] " Macpaul Lin
2011-04-27 10:11                 ` Detlev Zundel
2011-04-27 10:53                   ` Macpaul Lin
2011-04-27  2:16               ` [U-Boot] [PATCH v3] " Macpaul Lin
2011-04-27 14:44                 ` Detlev Zundel
2011-04-27 14:49                   ` Macpaul Lin
2011-04-30 20:27                 ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.