[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: RE: RFR(S): 8192978: Missing checks and small fixes in jdwp library
From: "Langer, Christoph" <christoph.langer () sap ! com>
Date: 2017-12-11 15:39:30
Message-ID: 99777f515c2747d783130dd122fd35de () sap ! com
[Download RAW message or body]
Hi Dan,
thanks for your help. I'll be more wary next time...
Best regards
Christoph
From: Daniel D. Daugherty [mailto:daniel.daugherty@oracle.com]
Sent: Montag, 11. Dezember 2017 14:10
To: Chris Plummer <chris.plummer@oracle.com>; Langer, Christoph \
<christoph.langer@sap.com>; serguei.spitsyn@oracle.com; \
serviceability-dev@openjdk.java.net
Subject: Re: RFR(S): 8192978: Missing checks and small fixes in jdwp library
Fixed.
Dan
On 12/11/17 2:53 AM, Chris Plummer wrote:
Hi Christoph,
I was just guilty of the same thing on Friday. Dan cleaned it up for me. See \
JDK-8191229. I believe the process is to change the Fix Version in the backport from \
10 to 11, re-open it, and then close as resolved "No an issue" (with a comment as to \
why this was done). For the main CR, change the Fix Version from 11 to 10, copy the \
HG Updater info from the backport, close as resolved "Fixed". I think you also need \
to set "Resolved In Build" to "team".
Chris
On 12/10/17 11:39 PM, Langer, Christoph wrote:
Hi guys,
I just pushed this: http://hg.openjdk.java.net/jdk/jdk/rev/8db54e2c453b
However, for some reason somebody set my original bug to Fix version 11 which I kinda \
ignored and now the system created a backport bug for 10 \
(https://bugs.openjdk.java.net/browse/JDK-8193305). Probably I should have corrected \
the fix version before I pushed. Now, shall I set the original bug \
https://bugs.openjdk.java.net/browse/JDK-8192978 to "Resolved" manually or wait for \
the system to do it? Any hints on that one?
Thanks
Christoph
From: Langer, Christoph
Sent: Samstag, 9. Dezember 2017 18:08
To: 'Chris Plummer' <chris.plummer@oracle.com><mailto:chris.plummer@oracle.com>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Cc: 'serguei.spitsyn@oracle.com<mailto:serguei.spitsyn@oracle.com>' \
<serguei.spitsyn@oracle.com><mailto:serguei.spitsyn@oracle.com>
Subject: RE: RFR(S): 8192978: Missing checks and small fixes in jdwp library
Hi Chris,
thanks for testing. I added the files with whitespace changes to the bug. I'll push \
this on Monday.
Best regards
Christoph
From: Chris Plummer [mailto:chris.plummer@oracle.com]
Sent: Samstag, 9. Dezember 2017 01:18
To: Langer, Christoph <christoph.langer@sap.com<mailto:christoph.langer@sap.com>>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Cc: Lindenmaier, Goetz <goetz.lindenmaier@sap.com<mailto:goetz.lindenmaier@sap.com>>
Subject: Re: RFR(S): 8192978: Missing checks and small fixes in jdwp library
All testing passed.
thanks,
Chris
On 12/8/17 1:07 PM, Langer, Christoph wrote:
Thanks in advance, Chris, for testing.
Please let me know the outcome when you are done.
Best regards
Christoph
From: Chris Plummer [mailto:chris.plummer@oracle.com]
Sent: Freitag, 8. Dezember 2017 21:52
To: Langer, Christoph <christoph.langer@sap.com><mailto:christoph.langer@sap.com>; \
serviceability-dev@openjdk.java.net<mailto:serviceability-dev@openjdk.java.net>
Cc: Lindenmaier, Goetz <goetz.lindenmaier@sap.com><mailto:goetz.lindenmaier@sap.com>
Subject: Re: RFR(S): 8192978: Missing checks and small fixes in jdwp library
Hi Christoph,
On 12/8/17 5:48 AM, Langer, Christoph wrote:
Hi,
this is a new webrev for 8192978. This change now only contains the coverity fixes.
In detail:
src/jdk.jdwp.agent/share/native/libjdwp/VirtualMachineImpl.c, static void \
writePaths(PacketOutputStream *out, char *string): strchr could be called with NULL \
argument because of assignment pos = psPos; in line 856 (last line of for loop). \
Proposal: check pos for NULL in head of for loop If I understand correctly how the \
code currently works, pos/psPos will be NULL on the last iteration of the loop, but \
we'll exit anyway since "i == npaths" by that point. So the NULL pos will never be \
referenced. I guess coverity is cluing in on the following section:
846 psPos = strchr(pos, ps[0]);
847 if ( psPos == NULL ) {
848 plen = (int)strlen(pos);
849 } else {
850 plen = (int)(psPos-pos);
851 psPos++;
852 }
We admit in line 847 that psPos can be NULL, but coverity can't read into the loop \
logic deep enough to recognize we'll also exit when this happens.
I guess your fix is fine, although technically unnecessary.
src/jdk.jdwp.agent/share/native/libjdwp/error_messages.c, static void \
vprint_message(FILE *fp, const char *prefix, const char *suffix, const char *format, \
va_list ap): potentially unterminated vsnprintf call. Proposal: terminate
Ok.
src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c, static jboolean \
synthesizeUnloadEvent(void *signatureVoid, void *envVoid): checking eventBag for \
NULL and then calling JDI_ASSERT only in that case is a bit dubious. It leads the \
coverity code scan tool to think that eventBag might be NULL when \
eventHelper_recordClassUnload is called which would eventually try to dereference a \
NULL eventbag and hence crash. Proposal: remove the NULL check but unconditionally \
assert.
This is the real fix for the changes in eventHelper.c in my former webrev.
Looks good. Thanks for looking deeper into this one.
src/jdk.jdwp.agent/share/native/libjdwp/log_messages.c, void log_message_end(const \
char *format, ...): potentially unterminated vsnprintf call. Proposal: terminate
Ok.
Furthermore I have 2 whitespace changes which I'd like to see because they would help \
me reducing the diff to our codebase and, furthermore, make the code looking nicer... \
a little ;-) Please list the files with just whitespace changes in the CR.
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8192978.2/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8192978.2/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8192978
I'm doing builds on the main platform and running jtreg tests.
I'm doing a pretty comprehensive round of serviceability testing myself with these \
changes and also 8193258. I'll let you know if there are any issues.
thanks,
Chris
Thanks in advance & Best regards
Christoph
[Attachment #3 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"> <head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
color:black;
mso-fareast-language:EN-US;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
tt
{mso-style-priority:99;
font-family:"Courier New";}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
span.EmailStyle19
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle22
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle24
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body bgcolor="white" lang="DE" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:windowtext">Hi Dan,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:windowtext"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="color:windowtext">thanks for your \
help. I’ll be more wary next time…<o:p></o:p></span></p> <p \
class="MsoNormal"><span lang="EN-US" \
style="color:windowtext"><o:p> </o:p></span></p> <p class="MsoNormal"><span \
lang="EN-US" style="color:windowtext">Best regards<o:p></o:p></span></p> <p \
class="MsoNormal"><span lang="EN-US" \
style="color:windowtext">Christoph<o:p></o:p></span></p> <p class="MsoNormal"><a \
name="_MailEndCompose"><span lang="EN-US"><o:p> </o:p></span></a></p> <span \
style="mso-bookmark:_MailEndCompose"></span> <div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-left:35.4pt"><b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE">From:</span></b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE"> Daniel D. Daugherty \
[mailto:daniel.daugherty@oracle.com] <br>
<b>Sent:</b> Montag, 11. Dezember 2017 14:10<br>
<b>To:</b> Chris Plummer <chris.plummer@oracle.com>; Langer, Christoph \
<christoph.langer@sap.com>; serguei.spitsyn@oracle.com; \
serviceability-dev@openjdk.java.net<br> <b>Subject:</b> Re: RFR(S): 8192978: Missing \
checks and small fixes in jdwp library<o:p></o:p></span></p> </div>
</div>
<p class="MsoNormal" style="margin-left:35.4pt"><o:p> </o:p></p>
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:35.4pt">
<tt><span style="font-size:10.0pt">Fixed.</span></tt><span \
style="font-size:10.0pt;font-family:"Courier New""><br> <br>
<tt>Dan</tt><br>
<br>
</span><span style="font-size:12.0pt;mso-fareast-language:DE"><o:p></o:p></span></p>
<div>
<p class="MsoNormal" style="margin-left:35.4pt">On 12/11/17 2:53 AM, Chris Plummer \
wrote:<o:p></o:p></p> </div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal" style="margin-left:35.4pt">Hi Christoph,<br>
<br>
I was just guilty of the same thing on Friday. Dan cleaned it up for me. See \
JDK-8191229. I believe the process is to change the Fix Version in the backport from \
10 to 11, re-open it, and then close as resolved "No an issue" (with a \
comment as to why this was done). For the main CR, change the Fix Version from 11 to \
10, copy the HG Updater info from the backport, close as resolved "Fixed". \
I think you also need to set "Resolved In Build" to "team".<br> \
<br> Chris<br>
<br>
On 12/10/17 11:39 PM, Langer, Christoph wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:35.4pt"><span style="color:windowtext">Hi \
guys,</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:35.4pt"><span \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:35.4pt"><span lang="EN-US" style="color:windowtext">I just pushed \
this: </span><a href="http://hg.openjdk.java.net/jdk/jdk/rev/8db54e2c453b"><span \
lang="EN-US">http://hg.openjdk.java.net/jdk/jdk/rev/8db54e2c453b</span></a><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:35.4pt"> <o:p></o:p></p>
<p class="MsoNormal" style="margin-left:35.4pt"><span lang="EN-US">However, for some \
reason somebody set my original bug to Fix version 11 which I kinda ignored and now \
the system created a backport bug for 10 (<a \
href="https://bugs.openjdk.java.net/browse/JDK-8193305">https://bugs.openjdk.java.net/browse/JDK-8193305</a>).
Probably I should have corrected the fix version before I pushed. Now, shall I set \
the original bug <a href="https://bugs.openjdk.java.net/browse/JDK-8192978">https://bugs.openjdk.java.net/browse/JDK-8192978</a> \
to “Resolved” manually or wait for the system to do it? Any hints on that \
one?</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:35.4pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:35.4pt"><span lang="EN-US">Thanks</span><o:p></o:p></p> <p \
class="MsoNormal" style="margin-left:35.4pt"><span \
lang="EN-US">Christoph</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:35.4pt"><span lang="EN-US"> </span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-left:70.8pt"><b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE">From:</span></b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE"> Langer, Christoph <br>
<b>Sent:</b> Samstag, 9. Dezember 2017 18:08<br>
<b>To:</b> 'Chris Plummer' <a \
href="mailto:chris.plummer@oracle.com"><chris.plummer@oracle.com></a>; <a \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Cc:</b> '<a href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>'
<a href="mailto:serguei.spitsyn@oracle.com"><serguei.spitsyn@oracle.com></a><br>
<b>Subject:</b> RE: RFR(S): 8192978: Missing checks and small fixes in jdwp \
library</span><o:p></o:p></p> </div>
</div>
<p class="MsoNormal" style="margin-left:70.8pt"> <o:p></o:p></p>
<p class="MsoNormal" style="margin-left:70.8pt"><span style="color:windowtext">Hi \
Chris,</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:70.8pt"><span \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:70.8pt"><span lang="EN-US" style="color:windowtext">thanks for \
testing. I added the files with whitespace changes to the bug. I’ll push this \
on Monday.</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:70.8pt"><span lang="EN-US" \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:70.8pt"><span lang="EN-US" style="color:windowtext">Best \
regards</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:70.8pt"><span \
lang="EN-US" style="color:windowtext">Christoph</span><o:p></o:p></p> <p \
class="MsoNormal" style="margin-left:70.8pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-left:106.2pt"><b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE">From:</span></b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE"> Chris Plummer [<a \
href="mailto:chris.plummer@oracle.com">mailto:chris.plummer@oracle.com</a>] <br>
<b>Sent:</b> Samstag, 9. Dezember 2017 01:18<br>
<b>To:</b> Langer, Christoph <<a \
href="mailto:christoph.langer@sap.com">christoph.langer@sap.com</a>>; <a \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Cc:</b> Lindenmaier, Goetz <<a \
href="mailto:goetz.lindenmaier@sap.com">goetz.lindenmaier@sap.com</a>><br> \
<b>Subject:</b> Re: RFR(S): 8192978: Missing checks and small fixes in jdwp \
library</span><o:p></o:p></p> </div>
</div>
<p class="MsoNormal" style="margin-left:106.2pt"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-left:106.2pt">All testing passed.<br>
<br>
thanks,<br>
<br>
Chris<br>
<br>
On 12/8/17 1:07 PM, Langer, Christoph wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:106.2pt"><span lang="EN-US" \
style="color:windowtext">Thanks in advance, Chris, for testing.</span><o:p></o:p></p> \
<p class="MsoNormal" style="margin-left:106.2pt"><span lang="EN-US" \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:106.2pt"><span lang="EN-US" style="color:windowtext">Please let me \
know the outcome when you are done.</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:106.2pt"><span lang="EN-US" \
style="color:windowtext"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:106.2pt"><span lang="EN-US" style="color:windowtext">Best \
regards</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:106.2pt"><span \
lang="EN-US" style="color:windowtext">Christoph</span><o:p></o:p></p> <p \
class="MsoNormal" style="margin-left:106.2pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-left:141.6pt"><b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE">From:</span></b><span lang="EN-US" \
style="color:windowtext;mso-fareast-language:DE"> Chris Plummer [<a \
href="mailto:chris.plummer@oracle.com">mailto:chris.plummer@oracle.com</a>] <br>
<b>Sent:</b> Freitag, 8. Dezember 2017 21:52<br>
<b>To:</b> Langer, Christoph <a \
href="mailto:christoph.langer@sap.com"><christoph.langer@sap.com></a>; <a \
href="mailto:serviceability-dev@openjdk.java.net">serviceability-dev@openjdk.java.net</a><br>
<b>Cc:</b> Lindenmaier, Goetz <a \
href="mailto:goetz.lindenmaier@sap.com"><goetz.lindenmaier@sap.com></a><br> \
<b>Subject:</b> Re: RFR(S): 8192978: Missing checks and small fixes in jdwp \
library</span><o:p></o:p></p> </div>
</div>
<p class="MsoNormal" style="margin-left:141.6pt"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="margin-left:141.6pt">Hi Christoph,<br>
<br>
On 12/8/17 5:48 AM, Langer, Christoph wrote:<o:p></o:p></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:141.6pt">Hi,<o:p></o:p></p>
<p class="MsoNormal" style="margin-left:141.6pt"> <o:p></o:p></p>
<p class="MsoNormal" style="margin-left:141.6pt"><span lang="EN-US">this is a new \
webrev for 8192978. This change now only contains the coverity \
fixes.</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US">In detail: <br> \
src/jdk.jdwp.agent/share/native/libjdwp/VirtualMachineImpl.c, static void \
writePaths(PacketOutputStream *out, char *string): <br>
strchr could be called with NULL argument because of \
assignment pos = psPos; in line 856 (last line of for loop). Proposal: check pos for \
NULL in head of for loop </span><o:p></o:p></p>
</blockquote>
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span style="font-size:12.0pt">If I understand correctly how the code currently \
works, pos/psPos will be NULL on the last iteration of the loop, but we'll exit \
anyway since "i == npaths" by that point. So the NULL pos will never be \
referenced. I guess coverity is cluing in on the following section:<br>
<br>
846 psPos = strchr(pos, \
ps[0]);<br> 847 if ( psPos == \
NULL ) {<br> 848 \
plen = (int)strlen(pos);<br> \
849 } else {<br> \
850 \
plen = (int)(psPos-pos);<br> \
851 \
psPos++;<br> 852 }<br>
<br>
We admit in line 847 that psPos can be NULL, but coverity can't read into the loop \
logic deep enough to recognize we'll also exit when this happens.<br> <br>
I guess your fix is fine, although technically unnecessary.<br>
<br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span lang="EN-US"><br>
<br>
<br>
</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> src/jdk.jdwp.agent/share/native/libjdwp/error_messages.c, static \
void vprint_message(FILE *fp, const char *prefix, const char *suffix, const char \
*format, va_list ap): <br>
potentially unterminated vsnprintf call. Proposal: terminate \
</span><o:p></o:p></p> </blockquote>
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span style="font-size:12.0pt">Ok.<br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span lang="EN-US"><br>
<br>
<br>
</span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US">src/jdk.jdwp.agent/share/native/libjdwp/eventHandler.c, static jboolean \
synthesizeUnloadEvent(void *signatureVoid, void *envVoid): <br>
checking eventBag for NULL and then calling JDI_ASSERT only \
in that case is a bit dubious.</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US"> It leads the \
coverity code scan tool to think that eventBag might be NULL when \
eventHelper_recordClassUnload is called</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US"> which would \
eventually try to dereference a NULL eventbag and hence crash.</span><o:p></o:p></p> \
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> Proposal: remove the NULL check but unconditionally \
assert. </span><o:p></o:p></p>
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US"> This is the real \
fix for the changes in eventHelper.c in my former webrev.</span><o:p></o:p></p> \
</blockquote> <p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span style="font-size:12.0pt">Looks good. Thanks for looking deeper into this one.
<br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:141.6pt"><span lang="EN-US"><br>
src/jdk.jdwp.agent/share/native/libjdwp/log_messages.c, void log_message_end(const \
char *format, ...): <br>
potentially unterminated vsnprintf call. Proposal: \
terminate</span><o:p></o:p></p> </blockquote>
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span style="font-size:12.0pt">Ok.<br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US">Furthermore I have 2 whitespace \
changes which I’d like to see because they would help me reducing the diff to \
our codebase and, furthermore, make the code looking nicer… a little \
;-)</span><o:p></o:p></p> </blockquote>
<p class="MsoNormal" \
style="mso-margin-top-alt:0cm;margin-right:0cm;margin-bottom:12.0pt;margin-left:141.6pt">
<span style="font-size:12.0pt">Please list the files with just whitespace changes in \
the CR.<br> <br>
<br>
<br>
</span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US">Webrev: <a \
href="http://cr.openjdk.java.net/%7Eclanger/webrevs/8192978.2/"> \
http://cr.openjdk.java.net/~clanger/webrevs/8192978.2/</a></span><o:p></o:p></p> <p \
class="MsoNormal" style="margin-left:141.6pt"><span lang="EN-US">Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-8192978"> \
https://bugs.openjdk.java.net/browse/JDK-8192978</a></span><o:p></o:p></p> <p \
class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US">I’m doing builds on the main \
platform and running jtreg tests.</span><o:p></o:p></p> </blockquote>
<p class="MsoNormal" style="margin-left:141.6pt"><span style="font-size:12.0pt">I'm \
doing a pretty comprehensive round of serviceability testing myself with these \
changes and also 8193258. I'll let you know if there are any issues.<br> <br>
thanks,<br>
<br>
Chris </span><o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US"> </span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US">Thanks in advance & Best \
regards</span><o:p></o:p></p> <p class="MsoNormal" style="margin-left:141.6pt"><span \
lang="EN-US">Christoph</span><o:p></o:p></p> <p class="MsoNormal" \
style="margin-left:141.6pt"><span lang="EN-US"> </span><o:p></o:p></p> \
</blockquote> <p style="margin-left:141.6pt"> <o:p></o:p></p>
</blockquote>
<p style="margin-left:106.2pt"> <o:p></o:p></p>
</blockquote>
<p style="margin-left:35.4pt"><o:p> </o:p></p>
</blockquote>
<p class="MsoNormal" style="margin-left:35.4pt"><span \
style="font-size:12.0pt;font-family:"Times New \
Roman",serif;mso-fareast-language:DE"><o:p> </o:p></span></p> </div>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic