public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
@ 2014-07-26  3:28 Chen Gang
  0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-07-26  3:28 UTC (permalink / raw)
  To: Jeff Law, cltang, sandra; +Cc: gcc-patches

The related strncpy() for custom_builtin_name[*] may set 5 none-zero
characters, which cause custom_builtin_name[*] none-zero terminated.
But error() assumes custom_builtin_name[*] is always zero terminated.

So add additional '\0' byte for custom_builtin_name[*].

ChangeLog:

	* config/nios2/nios2.c (custom_builtin_name): Let it always be
	zero terminated string.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 gcc/config/nios2/nios2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index a4e60c6..e4e005b 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -2510,7 +2510,7 @@ nios2_expand_fpu_builtin (tree exp, unsigned int code, rtx target)
    total of (3 + 1) * (1 + 3 + 9) == 52 custom builtin functions.
 */
 #define NUM_CUSTOM_BUILTINS ((3 + 1) * (1 + 3 + 9))
-static char custom_builtin_name[NUM_CUSTOM_BUILTINS][5];
+static char custom_builtin_name[NUM_CUSTOM_BUILTINS][6];
 
 static void
 nios2_init_custom_builtins (int start_code)
-- 
1.9.2.459.g68773ac

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  9:10     ` Chung-Lin Tang
@ 2014-07-26 11:45       ` Chen Gang
  0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-07-26 11:45 UTC (permalink / raw)
  To: Chung-Lin Tang, Chung-Lin Tang, Jeff Law, sandra; +Cc: gcc-patches

On 07/26/2014 05:07 PM, Chung-Lin Tang wrote:
> On 2014/7/26 03:33 PM, Chen Gang wrote:
>> On 07/26/2014 02:32 PM, Chung-Lin Tang wrote:
>>> On 14/7/26 11:28 AM, Chen Gang wrote:
>>>> The related strncpy() for custom_builtin_name[*] may set 5 none-zero
>>>> characters, which may cause custom_builtin_name[*] is none-zero
>>>> terminated.
>>>>
>>>> So add additional '\0' byte for custom_builtin_name[*].
>>>
>>> Where did you see this? Supposedly the snprintf of the custom function
>>> type string should at most be of 'xnxx' format; 4 characters at most,
>>
>> I guess, your 'xnxx' means "%cn%c%c", not "%sn%s%s" (which is current
>> implementation), also at present, "32 - n" is 17, not 4 for snprintf()
>> length limitation.
> 
> The use of %sn%s%s is intentional. That allows me to print "" directly
> using snprintf.
> 

OK, thanks.

> '32 - n' is to represent the rest of the 32-byte buffer, harmless
> really, as it is at most 4 chars.
> 
> Also, if I were to restrict it in the snprintf argument, it would be 5
> (including the null char) not 4.
> 

OK, thanks. And for me, I'd like to use 'sizeof(custom_builtin_name[0])'
instead of "32 - n" for snprintf().


>> If we are always sure it must be no more than 4 characters (at present,
>> it is, but in the future, I don't know). We can use strcpy() instead of
>> strncpy() for it -- that will let other readers no doubt.
>>
>> If we need let custom_builtin_name[*] not only zero terminated, but also
>> zero pad, we need pass '4' to strncpy() instead of '5', that also will
>> clear all doubts.
> 
> I don't understand what point you're trying to make here, really. As
> Andreas has noted in the other mail, strncpy does zero-termination
> automatically.
> 

strncpy will zero pad, but not be sure of the dest string will be zero
terminated, please "man strncpy" to get more details.

So for me, I'd like to use '4' instead of '5' for strncpy(), that will
clear all doubts (although, at present, it is not a bug, so this patch
can be skipped, if we are only focus on bug fix).


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  8:59     ` Andreas Schwab
@ 2014-07-26 10:43       ` Chen Gang
  0 siblings, 0 replies; 8+ messages in thread
From: Chen Gang @ 2014-07-26 10:43 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Chung-Lin Tang, Jeff Law, cltang, sandra, gcc-patches



On 07/26/2014 03:59 PM, Andreas Schwab wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
> 
>> If we need let custom_builtin_name[*] not only zero terminated, but also
>> zero pad, we need pass '4' to strncpy() instead of '5', that also will
>> clear all doubts.
> 
> If you pass 5 to strncpy to copy a string of at most 4 characters you
> get zero-termination for free.
> 

Yes. So at least, at present, it is no issue, so if we are focusing on
solving issues, this patch can be skipped.

And then I will continue discussing it in the other thread, please check
that.

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  7:59   ` Chen Gang
  2014-07-26  8:59     ` Andreas Schwab
@ 2014-07-26  9:10     ` Chung-Lin Tang
  2014-07-26 11:45       ` Chen Gang
  1 sibling, 1 reply; 8+ messages in thread
From: Chung-Lin Tang @ 2014-07-26  9:10 UTC (permalink / raw)
  To: Chen Gang, Chung-Lin Tang, Jeff Law, sandra; +Cc: gcc-patches

On 2014/7/26 03:33 PM, Chen Gang wrote:
> On 07/26/2014 02:32 PM, Chung-Lin Tang wrote:
>> On 14/7/26 11:28 AM, Chen Gang wrote:
>>> The related strncpy() for custom_builtin_name[*] may set 5 none-zero
>>> characters, which may cause custom_builtin_name[*] is none-zero
>>> terminated.
>>>
>>> So add additional '\0' byte for custom_builtin_name[*].
>>
>> Where did you see this? Supposedly the snprintf of the custom function
>> type string should at most be of 'xnxx' format; 4 characters at most,
> 
> I guess, your 'xnxx' means "%cn%c%c", not "%sn%s%s" (which is current
> implementation), also at present, "32 - n" is 17, not 4 for snprintf()
> length limitation.

The use of %sn%s%s is intentional. That allows me to print "" directly
using snprintf.

'32 - n' is to represent the rest of the 32-byte buffer, harmless
really, as it is at most 4 chars.

Also, if I were to restrict it in the snprintf argument, it would be 5
(including the null char) not 4.

> If we are always sure it must be no more than 4 characters (at present,
> it is, but in the future, I don't know). We can use strcpy() instead of
> strncpy() for it -- that will let other readers no doubt.
> 
> If we need let custom_builtin_name[*] not only zero terminated, but also
> zero pad, we need pass '4' to strncpy() instead of '5', that also will
> clear all doubts.

I don't understand what point you're trying to make here, really. As
Andreas has noted in the other mail, strncpy does zero-termination
automatically.

Chung-Lin

>> not 5.  Do you have a test case where the behavior you described appears?
>>
> 
> I find it by reading source code, for me, it is simple code, test is
> welcomed, but not mandatory.
> 
> But it really needs necessary discussion (for modification, and comments).

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  7:59   ` Chen Gang
@ 2014-07-26  8:59     ` Andreas Schwab
  2014-07-26 10:43       ` Chen Gang
  2014-07-26  9:10     ` Chung-Lin Tang
  1 sibling, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2014-07-26  8:59 UTC (permalink / raw)
  To: Chen Gang; +Cc: Chung-Lin Tang, Jeff Law, cltang, sandra, gcc-patches

Chen Gang <gang.chen.5i5j@gmail.com> writes:

> If we need let custom_builtin_name[*] not only zero terminated, but also
> zero pad, we need pass '4' to strncpy() instead of '5', that also will
> clear all doubts.

If you pass 5 to strncpy to copy a string of at most 4 characters you
get zero-termination for free.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  7:04 ` Chung-Lin Tang
@ 2014-07-26  7:59   ` Chen Gang
  2014-07-26  8:59     ` Andreas Schwab
  2014-07-26  9:10     ` Chung-Lin Tang
  0 siblings, 2 replies; 8+ messages in thread
From: Chen Gang @ 2014-07-26  7:59 UTC (permalink / raw)
  To: Chung-Lin Tang, Jeff Law, cltang, sandra; +Cc: gcc-patches

On 07/26/2014 02:32 PM, Chung-Lin Tang wrote:
> On 14/7/26 11:28 AM, Chen Gang wrote:
>> The related strncpy() for custom_builtin_name[*] may set 5 none-zero
>> characters, which may cause custom_builtin_name[*] is none-zero
>> terminated.
>>
>> So add additional '\0' byte for custom_builtin_name[*].
> 
> Where did you see this? Supposedly the snprintf of the custom function
> type string should at most be of 'xnxx' format; 4 characters at most,

I guess, your 'xnxx' means "%cn%c%c", not "%sn%s%s" (which is current
implementation), also at present, "32 - n" is 17, not 4 for snprintf()
length limitation.

If we are always sure it must be no more than 4 characters (at present,
it is, but in the future, I don't know). We can use strcpy() instead of
strncpy() for it -- that will let other readers no doubt.

If we need let custom_builtin_name[*] not only zero terminated, but also
zero pad, we need pass '4' to strncpy() instead of '5', that also will
clear all doubts.

> not 5.  Do you have a test case where the behavior you described appears?
> 

I find it by reading source code, for me, it is simple code, test is
welcomed, but not mandatory.

But it really needs necessary discussion (for modification, and comments).


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

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

* Re: [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
  2014-07-26  4:44 Chen Gang
@ 2014-07-26  7:04 ` Chung-Lin Tang
  2014-07-26  7:59   ` Chen Gang
  0 siblings, 1 reply; 8+ messages in thread
From: Chung-Lin Tang @ 2014-07-26  7:04 UTC (permalink / raw)
  To: Chen Gang, Jeff Law, cltang, sandra; +Cc: gcc-patches

On 14/7/26 11:28 AM, Chen Gang wrote:
> The related strncpy() for custom_builtin_name[*] may set 5 none-zero
> characters, which may cause custom_builtin_name[*] is none-zero
> terminated.
> 
> So add additional '\0' byte for custom_builtin_name[*].

Where did you see this? Supposedly the snprintf of the custom function
type string should at most be of 'xnxx' format; 4 characters at most,
not 5.  Do you have a test case where the behavior you described appears?

Thanks,
Chung-Lin

> ChangeLog:
> 
> 	* config/nios2/nios2.c (custom_builtin_name): Let it always be
> 	zero terminated string.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
>  gcc/config/nios2/nios2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
> index a4e60c6..e4e005b 100644
> --- a/gcc/config/nios2/nios2.c
> +++ b/gcc/config/nios2/nios2.c
> @@ -2510,7 +2510,7 @@ nios2_expand_fpu_builtin (tree exp, unsigned int code, rtx target)
>     total of (3 + 1) * (1 + 3 + 9) == 52 custom builtin functions.
>  */
>  #define NUM_CUSTOM_BUILTINS ((3 + 1) * (1 + 3 + 9))
> -static char custom_builtin_name[NUM_CUSTOM_BUILTINS][5];
> +static char custom_builtin_name[NUM_CUSTOM_BUILTINS][6];
>  
>  static void
>  nios2_init_custom_builtins (int start_code)
> 

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

* [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string
@ 2014-07-26  4:44 Chen Gang
  2014-07-26  7:04 ` Chung-Lin Tang
  0 siblings, 1 reply; 8+ messages in thread
From: Chen Gang @ 2014-07-26  4:44 UTC (permalink / raw)
  To: Jeff Law, cltang, sandra; +Cc: gcc-patches

The related strncpy() for custom_builtin_name[*] may set 5 none-zero
characters, which may cause custom_builtin_name[*] is none-zero
terminated.

So add additional '\0' byte for custom_builtin_name[*].

ChangeLog:

	* config/nios2/nios2.c (custom_builtin_name): Let it always be
	zero terminated string.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 gcc/config/nios2/nios2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index a4e60c6..e4e005b 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -2510,7 +2510,7 @@ nios2_expand_fpu_builtin (tree exp, unsigned int code, rtx target)
    total of (3 + 1) * (1 + 3 + 9) == 52 custom builtin functions.
 */
 #define NUM_CUSTOM_BUILTINS ((3 + 1) * (1 + 3 + 9))
-static char custom_builtin_name[NUM_CUSTOM_BUILTINS][5];
+static char custom_builtin_name[NUM_CUSTOM_BUILTINS][6];
 
 static void
 nios2_init_custom_builtins (int start_code)
-- 
1.9.2.459.g68773ac

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

end of thread, other threads:[~2014-07-26 10:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-26  3:28 [PATCH] gcc/config/nios2/nios2.c: Let custom_builtin_name[*] always be zero terminated string Chen Gang
2014-07-26  4:44 Chen Gang
2014-07-26  7:04 ` Chung-Lin Tang
2014-07-26  7:59   ` Chen Gang
2014-07-26  8:59     ` Andreas Schwab
2014-07-26 10:43       ` Chen Gang
2014-07-26  9:10     ` Chung-Lin Tang
2014-07-26 11:45       ` Chen Gang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).