[PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Ramsay Jones]

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 symbol.c                      | 10 ++++++++++
 validation/init-char-array1.c | 25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+)
 create mode 100644 validation/init-char-array1.c

diff --git a/symbol.c b/symbol.c
index 80a2f23..051a909 100644
--- a/symbol.c
+++ b/symbol.c
@@ -284,6 +284,15 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 	if (t->ctype.base_type == &int_type && t->ctype.modifiers & MOD_CHAR)
 		is_char = 1;
 
+	/* check for a parenthesized string: char x[] = ("string"); */
+	if (is_char && expr->type == EXPR_PREOP && expr->op == '(') {
+		struct expression *e = expr;
+		while (e && e->type == EXPR_PREOP && e->op == '(')
+			e = e->unop;
+		if (e && e->type == EXPR_STRING)
+			expr = e;
+	}
+
 	switch (expr->type) {
 	case EXPR_INITIALIZER: {
 		struct expression *entry;
@@ -310,6 +319,7 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 	case EXPR_STRING:
 		if (is_char)
 			nr = expr->string->length;
+		break;
 	default:
 		break;
 	}
diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
new file mode 100644
index 0000000..7427702
--- /dev/null
+++ b/validation/init-char-array1.c
@@ -0,0 +1,25 @@
+/*
+ * for array of char, ("...") as the initializer is an gcc language
+ * extension. check that a parenthesized string initializer is handled
+ * correctly and that -Wparen-string warns about it's use.
+ */
+static const char u[] = ("hello");
+static const char v[] = {"hello"};
+static const char w[] = "hello";
+static const char x[5] = "hello";
+
+static void f(void)
+{
+	char a[1/(sizeof(u) == 6)];
+	char b[1/(sizeof(v) == 6)];
+	char c[1/(sizeof(w) == 6)];
+	char d[1/(sizeof(x) == 5)];
+}
+/*
+ * check-name: parenthesized string initializer
+ * check-command: sparse -Wparen-string $file
+ *
+ * check-error-start
+init-char-array1.c:6:26: warning: array initialized from parenthesized string constant
+ * check-error-end
+ */
-- 
1.8.2

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Christopher Li]

On 05/16/2013 12:44 PM, Ramsay Jones wrote:

> diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
> new file mode 100644
> index 0000000..7427702
> --- /dev/null
> +++ b/validation/init-char-array1.c
> @@ -0,0 +1,25 @@
> +/*
> + * for array of char, ("...") as the initializer is an gcc language
> + * extension. check that a parenthesized string initializer is handled
> + * correctly and that -Wparen-string warns about it's use.
> + */
> +static const char u[] = ("hello");
> +static const char v[] = {"hello"};
> +static const char w[] = "hello";
> +static const char x[5] = "hello";

That seems not complete. I haven't found the official gcc rules
about parenthesized string. I just try the following forms and
gcc takes it.

char x1[] = { ("hello") };
char x2[] = { (("hello")) };

I guess the rules is that parenthesized string can appear in
any place where string was allowed.

Chris

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Chris Li]

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

On 05/18/2013 12:38 AM, Christopher Li wrote:
> I guess the rules is that parenthesized string can appear in
> any place where string was allowed.


I create an incremental patch to address the issue. Please
check if that works for you.

Chris


[-- Attachment #2: parenthesed-string-improve.patch --]
[-- Type: text/x-patch, Size: 2211 bytes --]

diff --git a/symbol.c b/symbol.c
index 051a909..a59a4ca 100644
--- a/symbol.c
+++ b/symbol.c
@@ -284,15 +284,6 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 	if (t->ctype.base_type == &int_type && t->ctype.modifiers & MOD_CHAR)
 		is_char = 1;
 
-	/* check for a parenthesized string: char x[] = ("string"); */
-	if (is_char && expr->type == EXPR_PREOP && expr->op == '(') {
-		struct expression *e = expr;
-		while (e && e->type == EXPR_PREOP && e->op == '(')
-			e = e->unop;
-		if (e && e->type == EXPR_STRING)
-			expr = e;
-	}
-
 	switch (expr->type) {
 	case EXPR_INITIALIZER: {
 		struct expression *entry;
@@ -305,6 +296,15 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 				if (entry->idx_to >= nr)
 					nr = entry->idx_to+1;
 				break;
+			case EXPR_PREOP: {
+				struct expression *e = entry;
+				while (e && e->type == EXPR_PREOP && e->op == '(')
+					e = e->unop;
+				if (!(e && e->type == EXPR_STRING))
+					break;
+				entry = e;
+				/* fall through to strings. */
+			}
 			case EXPR_STRING:
 				if (is_char)
 					str_len = entry->string->length;
@@ -316,6 +316,15 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 			nr = str_len;
 		break;
 	}
+	case EXPR_PREOP: {
+		struct expression *e = expr;
+		while (e && e->type == EXPR_PREOP && e->op == '(')
+			e = e->unop;
+		if (!(e && e->type == EXPR_STRING))
+			break;
+		expr = e;
+		/* fall through to strings. */
+	}
 	case EXPR_STRING:
 		if (is_char)
 			nr = expr->string->length;
diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
index 7427702..24fd8d8 100644
--- a/validation/init-char-array1.c
+++ b/validation/init-char-array1.c
@@ -5,6 +5,7 @@
  */
 static const char u[] = ("hello");
 static const char v[] = {"hello"};
+static const char v1[] = {("hello")};
 static const char w[] = "hello";
 static const char x[5] = "hello";
 
@@ -21,5 +22,6 @@ static void f(void)
  *
  * check-error-start
 init-char-array1.c:6:26: warning: array initialized from parenthesized string constant
+init-char-array1.c:8:28: warning: array initialized from parenthesized string constant
  * check-error-end
  */

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Ramsay Jones]

Chris Li wrote:
> On 05/18/2013 12:38 AM, Christopher Li wrote:
>> I guess the rules is that parenthesized string can appear in
>> any place where string was allowed.
> 
> 
> I create an incremental patch to address the issue. Please
> check if that works for you.

Unfortunately not. :(

In my git repo, I just happen to be on the pu branch:

  $ pwd
  /home/ramsay/git
  $ git describe
  v1.8.3-rc3-347-gac05152
  $ 

Using an older version of sparse (I don't know exactly which version;
although I have had a patch to add --version for some years, I wasn't
actually using a version with it applied!).

  $ make sparse >psp-out0 2>&1
  $ grep warning psp-out0 | wc -l
  7
  $ grep error psp-out0 | wc -l
  0
  $ 

Note that the severn warnings all relate to the glibc headers using
an transparent union for the various sockaddr types, for example:

      SP connect.c
  connect.c:272:40: warning: incorrect type in argument 2 (invalid types)
  connect.c:272:40:    expected union __CONST_SOCKADDR_ARG [usertype] __addr
  connect.c:272:40:    got struct sockaddr *ai_addr

Installing a new version of sparse:

  $ sparse --version
  v0.4.5-rc1
  $ make sparse >psp-out1 2>&1
  $ grep warning psp-out1 | wc -l
  9
  $ grep error psp-out1 | wc -l
  1
  $ diff psp-out0 psp-out1
  77a78
  > notes-merge.c:111:9: warning: too long initializer-string for array of char
  111a113
  > sha1_file.c:54:9: warning: too long initializer-string for array of char
  152a155
  > compat/regex/regex_internal.c:926:1: error: symbol 're_string_context_at' redeclared with different type (originally declared at compat/regex/regex_internal.h:434) - different modifiers
  $ 

Note that the two new warnings are fixed by my first patch and the
error is the second regression that I mentioned.

So, now introduce an "parenthesized string initializer", thus:

  $ vim gettext.h
  $ git diff
  diff --git a/gettext.h b/gettext.h
  index 7671d09..d11a413 100644
  --- a/gettext.h
  +++ b/gettext.h
  @@ -63,6 +63,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned lo
   }
   
   /* Mark msgid for translation but do not translate it. */
  -#define N_(msgid) msgid
  +#define N_(msgid) (msgid)
   
   #endif
  $ make sparse >psp-out2 2>&1
  $ grep warning psp-out2 | wc -l
  31
  $ grep error psp-out2 | wc -l
  2
  $ 

The new warnings all look like this:

  $ grep warning psp-out2 | head -3
  branch.c:216:1: warning: too long initializer-string for array of char
  branch.c:218:1: warning: too long initializer-string for array of char
  branch.c:220:1: warning: too long initializer-string for array of char
  $ 

The additional error is a simple syntax error:

  $ rm builtin/log.o
  $ make builtin/log.o
      CC builtin/log.o
  builtin/log.c:42: error: called object ‘"git log [<options>] [<revision range>] [[--] <path>...]\012"’ is not a function
  make: *** [builtin/log.o] Error 1
  $ make builtin/log.sp
      SP builtin/log.c
  builtin/log.c:42:9: error: not a function <noident>
  $ 

Now install a version of sparse with my three patches applied:

  $ sparse --version
  v0.4.5-rc1-3-g14eceaa
  $ make sparse >psp-out3 2>&1
  $ grep warning psp-out3 | wc -l
  7
  $ grep error psp-out3 | wc -l
  2
  $ 

So, all of the additional "too long initializer-string" warnings
are no longer issued.

Now add your additional patch on top:

  $ sparse --version
  v0.4.5-rc1-4-g5ef4e35
  $ make sparse >psp-out4 2>&1
  $ grep warning psp-out4 | wc -l
  177
  $ grep error psp-out4 | wc -l
  2
  $ 

All of the additional warnings, look like:

  $ grep warning psp-out4 | head -3
  archive.c:14:9: warning: excessive elements in array initializer
  archive.c:221:39: warning: excessive elements in array initializer
  attr.c:327:9: warning: excessive elements in array initializer
  $ 

Note that the warnings don't all involve the use of the gettext
'N_' macro. Also, many (I haven't checked them all) are actually
arrays of 'char *' (_not_ array of char). For example, from the
above list, archive.c:221 looks like this:

        const char *paths[] = { path, NULL };

HTH

ATB,
Ramsay Jones




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

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Chris Li]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On 05/20/2013 11:42 AM, Ramsay Jones wrote:

> Unfortunately not. :(

Sorry about that.

>         const char *paths[] = { path, NULL };

I see where I make the mistake. I need to drop the parentheses
only for char type. Can you please try this patch again? It is a
replacement patch of your patch No3. Not a delta. It works for the
the test case above.

Let's hope this one works.

Chris


[-- Attachment #2: 0001-symbol.c-Set-correct-size-of-array-from-parenthesize.patch --]
[-- Type: text/x-patch, Size: 2824 bytes --]

From 5a3bd40c1e9976c0896ad2d198e202a515dcf194 Mon Sep 17 00:00:00 2001
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Thu, 16 May 2013 20:44:11 +0100
Subject: [PATCH] symbol.c: Set correct size of array from parenthesized string
 initializer

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 symbol.c                      | 30 ++++++++++++++++++++++++++----
 validation/init-char-array1.c | 27 +++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)
 create mode 100644 validation/init-char-array1.c

diff --git a/symbol.c b/symbol.c
index 80a2f23..2ec6200 100644
--- a/symbol.c
+++ b/symbol.c
@@ -296,9 +296,21 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 				if (entry->idx_to >= nr)
 					nr = entry->idx_to+1;
 				break;
+			case EXPR_PREOP: {
+				struct expression *e = entry;
+				if (is_char) {
+					while (e && e->type == EXPR_PREOP && e->op == '(')
+						e = e->unop;
+					if (e && e->type == EXPR_STRING) {
+						entry = e;
 			case EXPR_STRING:
-				if (is_char)
-					str_len = entry->string->length;
+						if (is_char)
+							str_len = entry->string->length;
+					}
+
+
+				}
+			}
 			default:
 				nr++;
 			}
@@ -307,9 +319,19 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 			nr = str_len;
 		break;
 	}
+	case EXPR_PREOP:
+		if (is_char) { 
+			struct expression *e = expr;
+			while (e && e->type == EXPR_PREOP && e->op == '(')
+				e = e->unop;
+			if (e && e->type == EXPR_STRING) {
+				expr = e;
 	case EXPR_STRING:
-		if (is_char)
-			nr = expr->string->length;
+				if (is_char)
+					nr = expr->string->length;
+			}
+		}
+		break;
 	default:
 		break;
 	}
diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
new file mode 100644
index 0000000..24fd8d8
--- /dev/null
+++ b/validation/init-char-array1.c
@@ -0,0 +1,27 @@
+/*
+ * for array of char, ("...") as the initializer is an gcc language
+ * extension. check that a parenthesized string initializer is handled
+ * correctly and that -Wparen-string warns about it's use.
+ */
+static const char u[] = ("hello");
+static const char v[] = {"hello"};
+static const char v1[] = {("hello")};
+static const char w[] = "hello";
+static const char x[5] = "hello";
+
+static void f(void)
+{
+	char a[1/(sizeof(u) == 6)];
+	char b[1/(sizeof(v) == 6)];
+	char c[1/(sizeof(w) == 6)];
+	char d[1/(sizeof(x) == 5)];
+}
+/*
+ * check-name: parenthesized string initializer
+ * check-command: sparse -Wparen-string $file
+ *
+ * check-error-start
+init-char-array1.c:6:26: warning: array initialized from parenthesized string constant
+init-char-array1.c:8:28: warning: array initialized from parenthesized string constant
+ * check-error-end
+ */
-- 
1.8.1.4

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Ramsay Jones]

Chris Li wrote:
> On 05/20/2013 11:42 AM, Ramsay Jones wrote:
> 
>> Unfortunately not. :(
> 
> Sorry about that.
> 
>>         const char *paths[] = { path, NULL };
> 
> I see where I make the mistake. I need to drop the parentheses
> only for char type. Can you please try this patch again? It is a
> replacement patch of your patch No3. Not a delta. It works for the
> the test case above.
> 
> Let's hope this one works.

Yes, this patch works.

However, it made me go cross-eyed trying to follow the flow of
control. So, I created a new patch, added after the scissor mark
below, which hopefully makes the flow of control easier to follow.

Note that I originally wrote the code like:

    p = paren_string(expr);
    if (is_char && p)
        expr = p;

since sparse source seemed not to use assignments embedded into
conditions, but I stumbled across a few examples:

  $ git grep -n -e 'if (.*([a-zA-Z_]* = [a-zA-Z_]*'
  c2xml.c:173:            if ((base = builtin_typename(sym->ctype.base_type)) == N
  dissect.c:381:                  if ((expr = peek_preop(unop, '*')))
  dissect.c:388:                  if ((expr = peek_preop(unop, '&')))
  ptrlist.c:129:  if (!list || (nr = (last = list->prev)->nr) >= LIST_NODE_NR) {
  show-parse.c:289:       if ((typename = builtin_typename(sym))) {
  $

Also, note that I changed the test to add a check for the length
of the v1 array. Having said that, I had intended to rename the
variables in the test to u->y and a->e (this is a new test, after
all), but I forgot! sorry about that.

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Fri, 8 Apr 2011 23:38:30 +0100
Subject: [PATCH] symbol.c: Set correct size of array from parenthesized string initializer


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 symbol.c                      | 22 ++++++++++++++++++++++
 validation/init-char-array1.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 validation/init-char-array1.c

diff --git a/symbol.c b/symbol.c
index 80a2f23..55e1b47 100644
--- a/symbol.c
+++ b/symbol.c
@@ -269,8 +269,21 @@ void merge_type(struct symbol *sym, struct symbol *base_type)
 		merge_type(sym, sym->ctype.base_type);
 }
 
+static struct expression *paren_string(struct expression *expr)
+{
+	if (expr && expr->type == EXPR_PREOP && expr->op == '(') {
+		struct expression *e = expr;
+		while (e && e->type == EXPR_PREOP && e->op == '(')
+			e = e->unop;
+		if (e && e->type == EXPR_STRING)
+			return e;
+	}
+	return NULL;
+}
+
 static int count_array_initializer(struct symbol *t, struct expression *expr)
 {
+	struct expression *p;
 	int nr = 0;
 	int is_char = 0;
 
@@ -284,6 +297,10 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 	if (t->ctype.base_type == &int_type && t->ctype.modifiers & MOD_CHAR)
 		is_char = 1;
 
+	/* check for a parenthesized string: char x[] = ("string"); */
+	if (is_char && (p = paren_string(expr)))
+		expr = p;
+
 	switch (expr->type) {
 	case EXPR_INITIALIZER: {
 		struct expression *entry;
@@ -296,6 +313,10 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 				if (entry->idx_to >= nr)
 					nr = entry->idx_to+1;
 				break;
+			case EXPR_PREOP:
+				/* check for char x[] = {("string")}; */
+				if (is_char && (p = paren_string(entry)))
+					entry = p;
 			case EXPR_STRING:
 				if (is_char)
 					str_len = entry->string->length;
@@ -310,6 +331,7 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 	case EXPR_STRING:
 		if (is_char)
 			nr = expr->string->length;
+		break;
 	default:
 		break;
 	}
diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
new file mode 100644
index 0000000..bd4ed68
--- /dev/null
+++ b/validation/init-char-array1.c
@@ -0,0 +1,28 @@
+/*
+ * for array of char, ("...") as the initializer is an gcc language
+ * extension. check that a parenthesized string initializer is handled
+ * correctly and that -Wparen-string warns about it's use.
+ */
+static const char u[] = ("hello");
+static const char v[] = {"hello"};
+static const char v1[] = {("hello")};
+static const char w[] = "hello";
+static const char x[5] = "hello";
+
+static void f(void)
+{
+	char a[1/(sizeof(u) == 6)];
+	char b[1/(sizeof(v) == 6)];
+	char b1[1/(sizeof(v1) == 6)];
+	char c[1/(sizeof(w) == 6)];
+	char d[1/(sizeof(x) == 5)];
+}
+/*
+ * check-name: parenthesized string initializer
+ * check-command: sparse -Wparen-string $file
+ *
+ * check-error-start
+init-char-array1.c:6:26: warning: array initialized from parenthesized string constant
+init-char-array1.c:8:28: warning: array initialized from parenthesized string constant
+ * check-error-end
+ */
-- 
1.8.2

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Chris Li]

On Sat, May 25, 2013 at 09:01:30PM +0100, Ramsay Jones wrote:
> Yes, this patch works.
> 
> However, it made me go cross-eyed trying to follow the flow of
> control. So, I created a new patch, added after the scissor mark
> below, which hopefully makes the flow of control easier to follow.

As my patch, you should just read the "case EXPR_STRING" as a label,
that is easier to understand.

Unfortunately I think your patch is not doing the same thing as mine,
control flow wise. Please see the later comment on your code.

> Also, note that I changed the test to add a check for the length
> of the v1 array. Having said that, I had intended to rename the
> variables in the test to u->y and a->e (this is a new test, after
> all), but I forgot! sorry about that.

Can you submit a separate delta  for the test part of the change?
I can smash the patch on top your original one.

> +static struct expression *paren_string(struct expression *expr)

Paren sounds very much like "parent". I would make the function name
"parenthesized_string" or "extract_parenthesized_string".


>  
> +	/* check for a parenthesized string: char x[] = ("string"); */
> +	if (is_char && (p = paren_string(expr)))
> +		expr = p;
> +

I want to move this test to inside the switch statement.
The parenthesized string is a very rare case. Adding the test
here make the common case pay a price.


>  	switch (expr->type) {
>  	case EXPR_INITIALIZER: {
>  		struct expression *entry;
> @@ -296,6 +313,10 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
>  				if (entry->idx_to >= nr)
>  					nr = entry->idx_to+1;
>  				break;
> +			case EXPR_PREOP:
> +				/* check for char x[] = {("string")}; */
> +				if (is_char && (p = paren_string(entry)))
> +					entry = p;
>  			case EXPR_STRING:
>  				if (is_char)
>  					str_len = entry->string->length;

OK, here is the subtle bug. Consider the case expr->type ==  EXPR_PREOP
and is_char == 1 and paren_string(entry) return false.

Before apply your patch, it will go to default case.
Now after your patch, it will go to execute "str_len =
entry->string->length", even entry expression is not a string.

So your patch is doing some thing not intended. In order to correct
that, you can't always allow EXPR_PREOP case fall through to
EXPR_STRING. It need to depend on the test condition paren_string()
return true or not.

You can add goto statement to the switch statement to fix that.
It will make the control flow hard to read as well.

If you have better way to write it I am open to it.

Chris

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Ramsay Jones]

Chris Li wrote:
> On Sat, May 25, 2013 at 09:01:30PM +0100, Ramsay Jones wrote:
>> +static struct expression *paren_string(struct expression *expr)
> 
> Paren sounds very much like "parent". I would make the function name
> "parenthesized_string" or "extract_parenthesized_string".

OK, see below.

> 
>>  
>> +	/* check for a parenthesized string: char x[] = ("string"); */
>> +	if (is_char && (p = paren_string(expr)))
>> +		expr = p;
>> +
> 
> I want to move this test to inside the switch statement.
> The parenthesized string is a very rare case. Adding the test
> here make the common case pay a price.

OK

> 
>>  	switch (expr->type) {
>>  	case EXPR_INITIALIZER: {
>>  		struct expression *entry;
>> @@ -296,6 +313,10 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
>>  				if (entry->idx_to >= nr)
>>  					nr = entry->idx_to+1;
>>  				break;
>> +			case EXPR_PREOP:
>> +				/* check for char x[] = {("string")}; */
>> +				if (is_char && (p = paren_string(entry)))
>> +					entry = p;
>>  			case EXPR_STRING:
>>  				if (is_char)
>>  					str_len = entry->string->length;
> 
> OK, here is the subtle bug. Consider the case expr->type ==  EXPR_PREOP
> and is_char == 1 and paren_string(entry) return false.

Heh, indeed.

> 
> If you have better way to write it I am open to it.
> 

Well, I don't know that it's better; but the following patch (with or
without the diff applied) *reads* better to me. :-D

Note that the patch given below, after the scissors mark, keeps the
"fall through" into the string case, but I actually prefer not to do
so, and would add this (at least) on top:

  diff --git a/symbol.c b/symbol.c
  index 4e7b6e2..06e9596 100644
  --- a/symbol.c
  +++ b/symbol.c
  @@ -312,12 +312,12 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
   			case EXPR_PREOP:
   				/* check for: char x[] = {("string")}; */
   				p = parenthesized_string(entry);
  -				if (!(is_char && p)) {
  -					nr++;
  -					break;
  +				if (is_char && p) {
  +					entry = p;
  +					str_len = entry->string->length;
   				}
  -				entry = p;
  -				/* fall through to string */
  +				nr++;
  +				break;
   			case EXPR_STRING:
   				if (is_char)
   					str_len = entry->string->length;
  @@ -332,10 +332,11 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
   	case EXPR_PREOP:
   		/* check for parenthesized string: char x[] = ("string"); */
   		p = parenthesized_string(expr);
  -		if (!(is_char && p))
  -			break;
  -		expr = p;
  -		/* fall through to string */
  +		if (is_char && p) {
  +			expr = p;
  +			nr = expr->string->length;
  +		}
  +		break;
   	case EXPR_STRING:
   		if (is_char)
   			nr = expr->string->length;
  
Actually, I've just noticed that the last hunk above can be simplified
even further (there is no need to assign p to expr, just use p directly
in the assignment to nr).

[I would also consider making the string case self contained and not
falling through to the default case; i.e. add the "nr++;" and "break;"
statements.]

I have tested the patch below as-is, *and* with the above squashed in, on
Linux, cygwin and MinGW. Note, also, that the test has been fixed up.

Anyway, I will leave to choice of solution to you. ;-)

HTH

ATB,
Ramsay Jones


-- >8 --
From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Thu, 16 May 2013 20:44:11 +0100
Subject: [PATCH] symbol.c: Set correct size of array from parenthesized string initializer

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 symbol.c                      | 30 ++++++++++++++++++++++++++++++
 validation/init-char-array1.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 validation/init-char-array1.c

diff --git a/symbol.c b/symbol.c
index 80a2f23..4e7b6e2 100644
--- a/symbol.c
+++ b/symbol.c
@@ -269,8 +269,21 @@ void merge_type(struct symbol *sym, struct symbol *base_type)
 		merge_type(sym, sym->ctype.base_type);
 }
 
+static struct expression *parenthesized_string(struct expression *expr)
+{
+	if (expr && expr->type == EXPR_PREOP && expr->op == '(') {
+		struct expression *e = expr;
+		while (e && e->type == EXPR_PREOP && e->op == '(')
+			e = e->unop;
+		if (e && e->type == EXPR_STRING)
+			return e;
+	}
+	return NULL;
+}
+
 static int count_array_initializer(struct symbol *t, struct expression *expr)
 {
+	struct expression *p;
 	int nr = 0;
 	int is_char = 0;
 
@@ -296,6 +309,15 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 				if (entry->idx_to >= nr)
 					nr = entry->idx_to+1;
 				break;
+			case EXPR_PREOP:
+				/* check for: char x[] = {("string")}; */
+				p = parenthesized_string(entry);
+				if (!(is_char && p)) {
+					nr++;
+					break;
+				}
+				entry = p;
+				/* fall through to string */
 			case EXPR_STRING:
 				if (is_char)
 					str_len = entry->string->length;
@@ -307,9 +329,17 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
 			nr = str_len;
 		break;
 	}
+	case EXPR_PREOP:
+		/* check for parenthesized string: char x[] = ("string"); */
+		p = parenthesized_string(expr);
+		if (!(is_char && p))
+			break;
+		expr = p;
+		/* fall through to string */
 	case EXPR_STRING:
 		if (is_char)
 			nr = expr->string->length;
+		break;
 	default:
 		break;
 	}
diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
new file mode 100644
index 0000000..923741f
--- /dev/null
+++ b/validation/init-char-array1.c
@@ -0,0 +1,28 @@
+/*
+ * for array of char, ("...") as the initializer is an gcc language
+ * extension. check that a parenthesized string initializer is handled
+ * correctly and that -Wparen-string warns about it's use.
+ */
+static const char u[] = ("hello");
+static const char v[] = {"hello"};
+static const char w[] = {("hello")};
+static const char x[] = "hello";
+static const char y[5] = "hello";
+
+static void f(void)
+{
+	char a[1/(sizeof(u) == 6)];
+	char b[1/(sizeof(v) == 6)];
+	char c[1/(sizeof(w) == 6)];
+	char d[1/(sizeof(x) == 6)];
+	char e[1/(sizeof(y) == 5)];
+}
+/*
+ * check-name: parenthesized string initializer
+ * check-command: sparse -Wparen-string $file
+ *
+ * check-error-start
+init-char-array1.c:6:26: warning: array initialized from parenthesized string constant
+init-char-array1.c:8:27: warning: array initialized from parenthesized string constant
+ * check-error-end
+ */
-- 
1.8.3

Re: [PATCH 3/3] symbol.c: Set correct size of array from parenthesized string initializer

[Ramsay Jones]

Christopher Li wrote:
> On 05/16/2013 12:44 PM, Ramsay Jones wrote:
> 
>> diff --git a/validation/init-char-array1.c b/validation/init-char-array1.c
>> new file mode 100644
>> index 0000000..7427702
>> --- /dev/null
>> +++ b/validation/init-char-array1.c
>> @@ -0,0 +1,25 @@
>> +/*
>> + * for array of char, ("...") as the initializer is an gcc language
>> + * extension. check that a parenthesized string initializer is handled
>> + * correctly and that -Wparen-string warns about it's use.
>> + */
>> +static const char u[] = ("hello");
>> +static const char v[] = {"hello"};
>> +static const char w[] = "hello";
>> +static const char x[5] = "hello";
> 
> That seems not complete. I haven't found the official gcc rules
> about parenthesized string. I just try the following forms and
> gcc takes it.
> 
> char x1[] = { ("hello") };
> char x2[] = { (("hello")) };
> 
> I guess the rules is that parenthesized string can appear in
> any place where string was allowed.

Until i18n work started on git, I didn't know this was a gcc
extension at all! ;-) I looked at the gcc documentation available
to me at the time, and could not find it mentioned at all, let
alone as a gcc language extension. However, if it wasn't for the
-Wparen-string sparse warning, I wouldn't have suspected that
sparse was supposed to support it.

Also, I was slightly surprised that MSVC supported this syntax.
So, I looked at fixing sparse and came up with my patch. I then
found that tcc didn't support this syntax, which is not standard
C after all, so I submitted a patch to fix the git gettext 'N_'
macro which was the cause of the problem.

ATB,
Ramsay Jones