Hi Dan,
Thanks for the review!
> src/share/classes/sun/tools/jcmd/JCmd.java
> line 145: would split("$") work here? My memory is rusty but I
> think that catches all forms of line termination.
Good point, "$" should be more portable and easier to read. Will change.
> test/sun/tools/jcmd/jcmd-big-script.sh
> lines 46,61: I don't think '/dev/null' works on MKS and tests are
> still executed on MKS. grep around the testbase and see
> how other tests handle /dev/null.
Tests all passed on MKS already. Looks like /dev/null is supported.
[ UNIX-style /dev names for MKS Toolkit ]
http://www.mkssoftware.com/docs/man5/dev.5.asp
Strange, you can't "ls /dev" or "cd /dev", but you can "echo yourmom >
/dev/null" without any problem.
> line 48-49: please change to:
> status="$?"
> if [ "$status" != 0 ]; then
> echo "jcmd command returned non-zero exit code (status=$status). Failed."
Thanks. will change.
> line 54: please redirect output of 'grep' here to /dev/null or the
> appropriate bit bucket...
Thanks, will change as well.
Cheers,
-Buck
On 03/29/12 23:28, Daniel D. Daugherty wrote:
Resending... with David B on the reply...
On 3/28/12 10:28 PM, David Buck wrote:
Hi!
Please review my fix for the following bug :
[ Bug ID: 7154822 forward port fix for Bug 13645891 to JDK8 jcmd (1024
byte file size limit issue) ]
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154822
The issue is there is an arbitrary limit in the size of a script file
you pass to the jcmd command (via the -f option) of 1024 bytes. The
solution is for jcmd to break up the input file into individual lines
and send them one at a time to the jvm.
A similar fix has already been done for JRockit's jrcmd command and
will be released in R28.2.3.
[ jdk ]
http://cr.openjdk.java.net/~dbuck/7154822/webrev.00/
src/share/classes/sun/tools/jcmd/JCmd.java
line 145: would split("$") work here? My memory is rusty but I
think that catches all forms of line termination.
test/sun/tools/jcmd/dcmd-big-script.txt
No comments.
test/sun/tools/jcmd/jcmd-big-script.sh
lines 46,61: I don't think '/dev/null' works on MKS and tests are
still executed on MKS. grep around the testbase and see
how other tests handle /dev/null.
line 48-49: please change to:
status="$?"
if [ "$status" != 0 ]; then
echo "jcmd command returned non-zero exit code (status=$status). Failed."
line 54: please redirect output of 'grep' here to /dev/null or the
appropriate bit bucket...
Dan
All the default jprt tests and the jdk_tools tests were run and passed.
Cheers,
-Buck
