Hi all,
I pushed the change: https://hg.openjdk.java.net/jdk/jdk/rev/9f5b92d5a1b2
I hope PRAGMA_STRINGOP_TRUNCATION_IGNORED which introduced in this commit will
be used when similar issues occur in future.
Thanks for all your help! ありがとう！
Yasumasa
On 2019/10/17 13:54, Ioi Lam wrote:
On 10/16/19 9:25 PM, Yasumasa Suenaga wrote:
On 2019/10/17 13:21, David Holmes wrote:
On 17/10/2019 2:20 pm, Yasumasa Suenaga wrote:
On 2019/10/17 12:49, David Holmes wrote:
On 17/10/2019 12:45 pm, Yasumasa Suenaga wrote:
On 2019/10/17 11:39, David Holmes wrote:
On 17/10/2019 12:19 pm, Yasumasa Suenaga wrote:
On 2019/10/17 9:34, David Holmes wrote:
On 17/10/2019 10:07 am, serguei.spit...@oracle.com wrote:
Hi Yasumasa,
It looks good.
One tip:
+ // This code would be warned as "stringop-truncation" by GCC 8 or later.
+PRAGMA_DIAG_PUSH
+PRAGMA_STRINGOP_TRUNCATION_IGNORED
strncpy(buf, str, len);
+PRAGMA_DIAG_POP
I'd suggest to place the comment before the line with strncopy().
I'd suggest just dropping the comment as it adds nothing, further gcc 8 is not
flagging this use of strncpy.
The comment is suggested in [1].
Indeed stringop-truncation warning was not reported by GCC 8, but this option
introduced in it,
and I agree with Ioi to add the comment for other developers why this #pragma
is needed.
PRAGMA_STRINGOP_TRUNCATION_IGNORED
seems to pretty clearly state what the issue is to me. But if you want to add a
comment to make that more clear fine. But don't mention gcc 8 as it is not
relevant to the need for the pragma now.
I'd like to change the comment as below:
This code would be warned as "stringop-truncation" by modern GCC
Is it ok?
Grammatically it needs work. I propose
// This code can cause a "stringop-truncation" warning with gcc
"modern" will become inaccurate as time goes by.
Thanks!
To be clear that our code is correct, I will change the comment as below:
// This code can cause a "stringop-truncation" warning with gcc incorrectly
// This code can incorrectly cause a "stringop-truncation" warning with gcc
Assuming Ioi feels that is sufficient.
Thanks! and sorry for my English...
Yasumasa
I am happy with David's version. Let's get this over with :-)
Thanks to everyone, especially to Yasumasa's patience with this. お疲れ様です！
- Ioi
Thanks,
David
Yasumasa
Thanks,
David
Yasumasa
Thanks,
David
Yasumasa
[1]
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029578.html
Otherwise okay.
Thanks,
David
Not subject for re-review.
Thanks,
Serguei
On 10/16/19 16:25, Yasumasa Suenaga wrote:
(Re-send email because I could not send original email to serviceability-dev
and hotspot-compiler-dev)
Hi,
We discussed the fix for JDK-8232084 in [1], and I think we should fix it with
#pragma.
I uploaded new webrev. Could you review it?
JBS: https://bugs.openjdk.java.net/browse/JDK-8232084
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8232084/webrev.03/
This change has passed the tests on submit repo
(mach5-one-ysuenaga-JDK-8232084-1-20191016-1534-5969882).
Thanks,
Yasumasa
[1]
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029547.html
