Carsten,

The fix looks good.
I'll push it as soon as I get a time.

New bug filed:
  https://bugs.openjdk.java.net/browse/JDK-8141035

Feel free to update it if necessary.

Thanks,
Serguei


On 10/29/15 17:00, Carsten Varming wrote:
Dear Serguei and Steffan,

Thank you for the reviews. I have updated the webrev at http://cr.openjdk.java.net/~cvarming/jvmtiGen <http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen> . I haven't been able to log into JBS yet, so please go ahead and create bug id for this change.

Dan: Thank you for forwarding this email to the proper alias.

Carsten

On Thu, Oct 29, 2015 at 6:44 PM, serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com>> wrote:

    Carsten,

    I forgot to thank you for taking care about this issue!

    Thanks,
    Serguei


    On 10/29/15 15:41, serguei.spit...@oracle.com
    <mailto:serguei.spit...@oracle.com> wrote:

        Hi Carsten,

        The fix looks good.
        I share the Staffan's comments though.
        If understand correctly, you do not have an openjdk author
        status yet.

        I will sponsor your fix.
        Please, let me know the bug ID after you create one.


        Thanks,
        Serguei


        Please, let me know

        On 10/29/15 14:22, Staffan Larsen wrote:

            Carsten,

            This looks good with a few comments:

            1) If you make the “verbose” variable into a static field,
            you can avoid the final-copying.
            2) nit: Line 216: put "System.exit(1);” on it’s own line

            Oh, and create a bug: https://bugs.openjdk.java.net

            Thanks,
            /Staffan

                On 29 okt. 2015, at 14:54, Daniel D. Daugherty
                <daniel.daughe...@oracle.com
                <mailto:daniel.daughe...@oracle.com>> wrote:

                JVM/TI belongs to the Serviceability team so adding
                serviceability-dev@...

                Dan



                On 10/28/15 8:45 PM, Carsten Varming wrote:

                    webrev:
                    http://cr.openjdk.java.net/~cvarming/jvmtiGen/
                    <http://cr.openjdk.java.net/%7Ecvarming/jvmtiGen/>
                    bug: ?

                    jvmtiGen is used to process a number of xml and
                    xslt files in OpenJDK.
                    Currently jvmtiGen exits with exit code 0
                    regardless of its success. This
                    causes make to often consider a target finished
                    when in fact the target
                    failed. It also leads to funny error checking
                    after the execution of
                    jvmtiGen. For instance, in many trace.make
                    files[*] a test for the
                    existence of the output file is carried out after
                    the completion of
                    jvmtiGen. In a clean working repository that test
                    is equivalent to jvmtiGen
                    exiting with a proper exit failure code on
                    failure, but in a dirty working
                    repository the target file might just be
                    pre-existing. This causes
                    unnecessary pain when working with files processed
                    by jvmtiGen.

                    In this change I chose to exit with exit code 1
                    whenever a failure is
                    detected, be it a dtd validation failure, an IO
                    failure, or something else
                    entirely. This halts the building of OpenJDK on
                    failures and ultimately
                    makes development easier. I also added a verbose
                    option such that warnings
                    from the xml parser and dtd checker can be printed
                    on stderr if desired.
                    Finally, I changed all the error message printing
                    to stderr. :-)

                    Let me know what you think.

                    BTW. This is the first time I tried the webrev
                    system, so hopefully it all
                    looks good. I havn't figured out how to create a
                    bug yet, whence the
                    question mark.

                    I wasn't sure if hotspot-runtime-dev is the right
                    email alias. Please let
                    me know if there is a more appropriate alias for
                    this email.

                    [*] Why are so many of the non-shared makefiles
                    almost identical?





Reply via email to