Patchwork [18/25] drivers/serial/suncore.c: Use static const char arrays

login
register
mail settings
Submitter Joe Perches
Date Sept. 13, 2010, 7:47 p.m.
Message ID <0ab674aa32a5fcc364982c78efdf44864a61523b.1284406639.git.joe@perches.com>
Download mbox | patch
Permalink /patch/64634/
State Rejected
Delegated to: David Miller
Headers show

Comments

Joe Perches - Sept. 13, 2010, 7:47 p.m.
Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/serial/suncore.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Joe Perches - Sept. 13, 2010, 8:22 p.m.
On Mon, 2010-09-13 at 21:36 +0100, Alan Cox wrote:
> On Mon, 13 Sep 2010 12:47:56 -0700
> Joe Perches <joe@perches.com> wrote:
> > diff --git a/drivers/serial/suncore.c b/drivers/serial/suncore.c
> > @@ -106,7 +106,7 @@ void sunserial_console_termios(struct console *con, struct device_node *uart_dp)
> >  		if (of_console_options)
> >  			c = *of_console_options;
> >  
> > -		mode_prop[3] = c;
> > +		sprintf(mode_prop, "tty%c-mode", c);
> Whats the point of all this. It's trivial code being replaced by
> something complicated, harder to understand and much slower ?

There are many cases of char foo[] = "bar" that
should be actually be [static] const char foo[] etc.
The patch set was done to change those cases.

This case isn't actually any sort of saving.

As I just wrote in 5/25 to Jonathan Cameron:

It tries to standardize the style use and it avoids possible
future checkpatch warnings of:
	char foo[] = "bar"
char array could possibly be static const.

There was another use with "%1.1d" somewhere.

The end result is the same, so I don't really care much
if this sort of change is applied or not.  The possible
checkpatch message could just be considered noise but
Mike Frysinger seemed to prefer it, so I thought I could
try to accommodate him.

cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Cox - Sept. 13, 2010, 8:36 p.m.
On Mon, 13 Sep 2010 12:47:56 -0700
Joe Perches <joe@perches.com> wrote:

> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/serial/suncore.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/serial/suncore.c b/drivers/serial/suncore.c
> index 6381a02..f2a5b54 100644
> --- a/drivers/serial/suncore.c
> +++ b/drivers/serial/suncore.c
> @@ -84,8 +84,8 @@ EXPORT_SYMBOL(sunserial_console_match);
>  
>  void sunserial_console_termios(struct console *con, struct device_node *uart_dp)
>  {
> +	char mode_prop[sizeof("ttyX-mode")];
>  	const char *mode, *s;
> -	char mode_prop[] = "ttyX-mode";
>  	int baud, bits, stop, cflag;
>  	char parity;
>  
> @@ -106,7 +106,7 @@ void sunserial_console_termios(struct console *con, struct device_node *uart_dp)
>  		if (of_console_options)
>  			c = *of_console_options;
>  
> -		mode_prop[3] = c;
> +		sprintf(mode_prop, "tty%c-mode", c);

Whats the point of all this. It's trivial code being replaced by
something complicated, harder to understand and much slower ?
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nick Bowler - Sept. 13, 2010, 9:11 p.m.
On 2010-09-13 13:22 -0700, Joe Perches wrote:
> It tries to standardize the style use and it avoids possible
> future checkpatch warnings of:
> 	char foo[] = "bar"
> char array could possibly be static const.

Whoa, back up a second here.  I've seen patches to avoid questionable
checkpatch warnings before, but I think this is the first time I've seen
a patch to avoid *hypothetical checkpatch warnings that don't even
exist*!
Joe Perches - Sept. 13, 2010, 9:17 p.m.
On Mon, 2010-09-13 at 17:11 -0400, Nick Bowler wrote:
> On 2010-09-13 13:22 -0700, Joe Perches wrote:
> > It tries to standardize the style use and it avoids possible
> > future checkpatch warnings of:
> > 	char foo[] = "bar"
> > char array could possibly be static const.
> Whoa, back up a second here.  I've seen patches to avoid questionable
> checkpatch warnings before, but I think this is the first time I've seen
> a patch to avoid *hypothetical checkpatch warnings that don't even
> exist*!

Pretty cool huh?  cheers, Joe

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/serial/suncore.c b/drivers/serial/suncore.c
index 6381a02..f2a5b54 100644
--- a/drivers/serial/suncore.c
+++ b/drivers/serial/suncore.c
@@ -84,8 +84,8 @@  EXPORT_SYMBOL(sunserial_console_match);
 
 void sunserial_console_termios(struct console *con, struct device_node *uart_dp)
 {
+	char mode_prop[sizeof("ttyX-mode")];
 	const char *mode, *s;
-	char mode_prop[] = "ttyX-mode";
 	int baud, bits, stop, cflag;
 	char parity;
 
@@ -106,7 +106,7 @@  void sunserial_console_termios(struct console *con, struct device_node *uart_dp)
 		if (of_console_options)
 			c = *of_console_options;
 
-		mode_prop[3] = c;
+		sprintf(mode_prop, "tty%c-mode", c);
 
 		dp = of_find_node_by_path("/options");
 		mode = of_get_property(dp, mode_prop, NULL);