<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix"><br>
      Hi,<br>
      <br>
      Yes the removal of System.exit didn't take the error value into
      account.  8010278 removed the System.exit so these Tool classes
      were usable by other tools.  I suppose we can use it in main() as
      if we're there we should be running as a standalone app.<br>
      <br>
      The execute method seems like a nice simplification for the
      Tools.  An external app that wants to call one of the Tools can't
      use it, as it may call System.exit (and you don't want to make
      execute return an int as then every Tool class has to have more
      logic in its main() ).<br>
      <br>
      nits in Tool.java<br>
      115 {  should be on the same line as execute definition begins.<br>
      120 } finally { on one line<br>
      <br>
      I don't think stop() should become private, an existing app that
      used "public void start()" would expect to call a public stop().<br>
      <br>
      There are a few other files that might want looking at to get them
      returning an error code: CLHSDB and HSDB (CommandProcessor looks
      OK, it only had System.exit(0) removed.)  That could be a separate
      bug...<br>
      <br>
      Thanks!<br>
      Kevin<br>
      <br>
      <br>
      On 10/10/13 12:49, Staffan Larsen wrote:<br>
    </div>
    <blockquote
      cite="mid:423EB291-3E36-41F7-86F6-6362D0B49839@oracle.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=ISO-8859-1">
      <div>Thanks for doing this!</div>
      <div><br>
      </div>
      Tool.java:116 - shouldn't the default return value be 1? In case
      start() throws an exception for some reason.
      <div><br>
      </div>
      <div>Tool.java: I find the start(String[] args), start(),
        startInternal() methods confusing in naming and usage. Not
        directly related to your change of course, just a comment on the
        code.</div>
      <div><br>
      </div>
      <div>/Staffan<br>
        <div><br>
        </div>
        <div><br>
          <div>
            <div>On 10 okt 2013, at 13:38, Staffan Larsen <<a
                moz-do-not-send="true"
                href="mailto:staffan.larsen@oracle.com">staffan.larsen@oracle.com</a>>
              wrote:</div>
            <br class="Apple-interchange-newline">
            <blockquote type="cite">
              <meta http-equiv="Content-Type" content="text/html;
                charset=ISO-8859-1">
              <div style="word-wrap: break-word; -webkit-nbsp-mode:
                space; -webkit-line-break: after-white-space; ">Clicking
                on the link in this email takes me to the wrong webrev.
                The correct URL is in the text: <a
                  moz-do-not-send="true"
                  href="http://cr.openjdk.java.net/%7Eallwin/farvidss/8025638/webrev.00/">http://cr.openjdk.java.net/~allwin/farvidss/8025638/webrev.00/</a>
                <div><br>
                </div>
                <div>/Staffan</div>
                <div><br>
                  <div>
                    <div>On 10 okt 2013, at 12:42, Fredrik Arvidsson
                      <<a moz-do-not-send="true"
                        href="mailto:fredrik.arvidsson@oracle.com">fredrik.arvidsson@oracle.com</a>>
                      wrote:</div>
                    <br class="Apple-interchange-newline">
                    <blockquote type="cite">
                      <meta http-equiv="content-type"
                        content="text/html; charset=ISO-8859-1">
                      <div text="#000000" bgcolor="#FFFFFF"> Hi<br>
                        <br>
                        Please help me to review the changes below:<br>
                        <br>
                        Jira case: <a moz-do-not-send="true"
                          href="https://bugs.openjdk.java.net/browse/JDK-8024423">https://bugs.openjdk.java.net/browse/JDK-8025638</a><br>
                        Webrev: <a moz-do-not-send="true"
                          href="http://cr.openjdk.java.net/%7Eallwin/farvidss/8024423/webrev.00/">http://cr.openjdk.java.net/~allwin/farvidss/8025638/webrev.00/</a><br>
                        <br>
                        About this change.<br>
                        A previous change (<a moz-do-not-send="true"
                          href="https://bugs.openjdk.java.net/browse/JDK-8010278">https://bugs.openjdk.java.net/browse/JDK-8010278</a>)
                        in the Tool.java class caused any tool deriving
                        from this base class return the wrong value to
                        the caller when failing. <br>
                        Changes were made to the Tool.java class and to
                        the derived tool implementation classes to
                        handle errors/exceptions during execution and
                        ensure that the tool returns 1 to the caller if
                        it fails, and 0 if it succeeds. <br>
                        <br>
                        Previously failed Aurora tests have been run
                        using UTE and verified to PASS.<br>
                        <br>
                        Cheers<br>
                        /Fredrik </div>
                    </blockquote>
                  </div>
                  <br>
                </div>
              </div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>