diff mbox

[v3] gcc/c-family/c-cppbuiltin.c: Let buffer enough to print host wide integer value

Message ID 5474DB2F.3010702@gmail.com
State New
Headers show

Commit Message

Chen Gang Nov. 25, 2014, 7:40 p.m. UTC 
The original length 18 is not enough for HOST_WIDE_INT printing, need
use 20 instead of.

Also need additional bytes for printing related prefix and suffix, and
give a related check.

It passes testsuite under fedora 20 x86_64-unknown-linux-gnu.


2014-11-26  Chen Gang <gang.chen.5i5j@gmail.com>

	* c-family/c-cppbuiltin.c (builtin_define_with_int_value): Let
	buffer enough to print host wide integer value.
---
 gcc/c-family/c-cppbuiltin.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Joseph Myers Nov. 26, 2014, 12:31 a.m. UTC | #1 
On Wed, 26 Nov 2014, Chen Gang wrote:

> +  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
> +	      || wi::fits_to_tree_p (value, short_integer_type_node)
> +	      || wi::fits_to_tree_p (value, integer_type_node)
> +	      || wi::fits_to_tree_p (value, long_integer_type_node)
> +	      || wi::fits_to_tree_p (value, long_long_integer_type_node));

It doesn't make sense to check for char or short, since you can't write a 
constant of one of those types.  And it doesn't make sense to check for 
int or long when checking for long long, as the ranges of int and long are 
subsets of that of long long.  So just check long long here.

> +  buf = (char *) alloca (strlen (macro) + vlen + extra);
> +
> +  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
> +	   macro,
> +	   value < 0 ? "(" : "",
> +	   value,
> +	   wi::fits_to_tree_p (value, char_type_node)
> +	     || wi::fits_to_tree_p (value, short_integer_type_node)
> +	     || wi::fits_to_tree_p (value, integer_type_node)

No need to check for char or short here.
Joseph Myers Nov. 26, 2014, 1:38 a.m. UTC | #2 
On Wed, 26 Nov 2014, Chen Gang wrote:

> And I have no any ideas about the attachments in your reply mail. If it
> is really related with this thread, please let me know.

I don't understand what attachments you are referring to.
Chen Gang Nov. 26, 2014, 1:41 a.m. UTC | #3 
On 11/26/14 8:31, Joseph Myers wrote:
> On Wed, 26 Nov 2014, Chen Gang wrote:
> 
>> +  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
>> +	      || wi::fits_to_tree_p (value, short_integer_type_node)
>> +	      || wi::fits_to_tree_p (value, integer_type_node)
>> +	      || wi::fits_to_tree_p (value, long_integer_type_node)
>> +	      || wi::fits_to_tree_p (value, long_long_integer_type_node));
> 
> It doesn't make sense to check for char or short, since you can't write a 
> constant of one of those types.  And it doesn't make sense to check for 
> int or long when checking for long long, as the ranges of int and long are 
> subsets of that of long long.  So just check long long here.
> 
>> +  buf = (char *) alloca (strlen (macro) + vlen + extra);
>> +
>> +  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
>> +	   macro,
>> +	   value < 0 ? "(" : "",
>> +	   value,
>> +	   wi::fits_to_tree_p (value, char_type_node)
>> +	     || wi::fits_to_tree_p (value, short_integer_type_node)
>> +	     || wi::fits_to_tree_p (value, integer_type_node)
> 
> No need to check for char or short here.
> 

Thank you very much for your patient work, I shall send patch v4 for it
within 2 days.

And I have no any ideas about the attachments in your reply mail. If it
is really related with this thread, please let me know.

Thanks.
Chen Gang Nov. 26, 2014, 1:52 a.m. UTC | #4 
On 11/26/14 9:38, Joseph Myers wrote:
> On Wed, 26 Nov 2014, Chen Gang wrote:
> 
>> And I have no any ideas about the attachments in your reply mail. If it
>> is really related with this thread, please let me know.
> 
> I don't understand what attachments you are referring to.
> 

Oh, sorry, my Thunderbird mail client make a trick to me.

Thanks.
Jakub Jelinek Nov. 26, 2014, 7:33 a.m. UTC | #5 
On Wed, Nov 26, 2014 at 09:41:16AM +0800, Chen Gang wrote:
> On 11/26/14 8:31, Joseph Myers wrote:
> > On Wed, 26 Nov 2014, Chen Gang wrote:
> > 
> >> +  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
> >> +	      || wi::fits_to_tree_p (value, short_integer_type_node)
> >> +	      || wi::fits_to_tree_p (value, integer_type_node)
> >> +	      || wi::fits_to_tree_p (value, long_integer_type_node)
> >> +	      || wi::fits_to_tree_p (value, long_long_integer_type_node));
> > 
> > It doesn't make sense to check for char or short, since you can't write a 
> > constant of one of those types.  And it doesn't make sense to check for 
> > int or long when checking for long long, as the ranges of int and long are 
> > subsets of that of long long.  So just check long long here.
> > 
> >> +  buf = (char *) alloca (strlen (macro) + vlen + extra);
> >> +
> >> +  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",

Oh, and please use spaces around HOST_WIDE_INT_PRINT_DEC etc.,
with C++11 user defined literals it is always better to have whitespace
in there.

	Jakub
Chen Gang Nov. 26, 2014, 9:34 a.m. UTC | #6 
On 11/26/14 15:33, Jakub Jelinek wrote:
> On Wed, Nov 26, 2014 at 09:41:16AM +0800, Chen Gang wrote:
>> On 11/26/14 8:31, Joseph Myers wrote:
>>> On Wed, 26 Nov 2014, Chen Gang wrote:
>>>
>>>> +  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
>>>> +	      || wi::fits_to_tree_p (value, short_integer_type_node)
>>>> +	      || wi::fits_to_tree_p (value, integer_type_node)
>>>> +	      || wi::fits_to_tree_p (value, long_integer_type_node)
>>>> +	      || wi::fits_to_tree_p (value, long_long_integer_type_node));
>>>
>>> It doesn't make sense to check for char or short, since you can't write a 
>>> constant of one of those types.  And it doesn't make sense to check for 
>>> int or long when checking for long long, as the ranges of int and long are 
>>> subsets of that of long long.  So just check long long here.
>>>
>>>> +  buf = (char *) alloca (strlen (macro) + vlen + extra);
>>>> +
>>>> +  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
> 
> Oh, and please use spaces around HOST_WIDE_INT_PRINT_DEC etc.,
> with C++11 user defined literals it is always better to have whitespace
> in there.
> 

OK, thanks, I shall notice about it when send patch v4.


Thanks.
diff mbox

Patch

diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index c571d1b..b1b96fb 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -1366,14 +1366,28 @@  static void
 builtin_define_with_int_value (const char *macro, HOST_WIDE_INT value)
 {
   char *buf;
-  size_t mlen = strlen (macro);
-  size_t vlen = 18;
-  size_t extra = 2; /* space for = and NUL.  */
-
-  buf = (char *) alloca (mlen + vlen + extra);
-  memcpy (buf, macro, mlen);
-  buf[mlen] = '=';
-  sprintf (buf + mlen + 1, HOST_WIDE_INT_PRINT_DEC, value);
+  size_t vlen = 20; /* maximize value length: -9223372036854775807 */
+  size_t extra = 6; /* space for =, NUL, (, ), and L L. */
+
+  gcc_assert (wi::fits_to_tree_p (value, char_type_node)
+	      || wi::fits_to_tree_p (value, short_integer_type_node)
+	      || wi::fits_to_tree_p (value, integer_type_node)
+	      || wi::fits_to_tree_p (value, long_integer_type_node)
+	      || wi::fits_to_tree_p (value, long_long_integer_type_node));
+
+  buf = (char *) alloca (strlen (macro) + vlen + extra);
+
+  sprintf (buf, "%s=%s"HOST_WIDE_INT_PRINT_DEC"%s%s",
+	   macro,
+	   value < 0 ? "(" : "",
+	   value,
+	   wi::fits_to_tree_p (value, char_type_node)
+	     || wi::fits_to_tree_p (value, short_integer_type_node)
+	     || wi::fits_to_tree_p (value, integer_type_node)
+	   ? ""
+	   : wi::fits_to_tree_p (value, long_integer_type_node)
+	     ? "L" : "LL",
+	   value < 0 ? ")" : "");
 
   cpp_define (parse_in, buf);
 }