<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 Fredrik,<br>
      <br>
      Yes, I think that looks good.<br>
      <br>
      Thanks!<br>
      Kevin<br>
      <br>
      <br>
      On 15/10/13 13:48, Fredrik Arvidsson wrote:<br>
    </div>
    <blockquote cite="mid:525D398E.3010802@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      I have created a new review of this with the changes suggested.<br>
      <br>
      The review can be found here:<br>
      <a moz-do-not-send="true"
        href="http://cr.openjdk.java.net/%7Eallwin/farvidss/8025638/webrev.01/">http://cr.openjdk.java.net/~allwin/farvidss/8025638/webrev.01/</a><br>
      <br>
      /F<br>
      <br>
      <div class="moz-cite-prefix">On 2013-10-10 16:49, Kevin Walls
        wrote:<br>
      </div>
      <blockquote cite="mid:5256BE5C.4070406@oracle.com" type="cite">
        <meta content="text/html; charset=ISO-8859-1"
          http-equiv="Content-Type">
        <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>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>