<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi David,<br>
    <br>
    Thanks for your comments.  Please see my interspersed comments.<br>
    <br>
    Also, I posted a modified webrev at <a
      href="http://cr.openjdk.java.net/%7Ehseigel/bug_7102489_2/">http://cr.openjdk.java.net/~hseigel/bug_7102489_2/</a>
    that has 32 and 64 definitions of JLONG_FORMAT in
    .../launcher/java_md.h and 2013 copyrights.<br>
    <br>
    Thanks, Harold<br>
    <br>
    On 1/16/2013 1:53 AM, David Holmes wrote:
    <blockquote cite="mid:50F64E60.3030807@oracle.com" type="cite">On
      16/01/2013 9:12 AM, Coleen Phillimore wrote:
      <br>
      <blockquote type="cite">
        <br>
        I would really be worried about changing the definition in
        hotspot of
        <br>
        jlong.  That seems very risky.
        <br>
      </blockquote>
      <br>
      No risk. jlong is a 64-bit signed value regardless. And int64_t
      will be either long or "long long" as appropriate.
      <br>
    </blockquote>
    I tried changing the type of jlong to int64_t and encountered
    compilation errors on 64 bit Linux, because I had added "#include
    &lt;stdint.h&gt;" to jni_x86.h.   The header file was needed because
    it contains the definition of int64_t.  An example error was:<br>
    <blockquote>
      <pre>.../src/share/vm/memory/allocation.hpp:
    In member function void Arena::check_for_overflow(size_t, const char*) const:
    .../src/share/vm/memory/allocation.hpp:340:
    error: UINTPTR_MAX was not declared in this scope</pre>
    </blockquote>
    Type jlong is defined as long in the jni_md.h file that we ship on
    Mac OS.  If we changed jlong to int64_t everywhere, then users may
    suddenly encounter compilation errors, also.<br>
    <blockquote cite="mid:50F64E60.3030807@oracle.com" type="cite">
      <br>
      <blockquote type="cite">I have to admit that this change looks
        <br>
        very clean to me.  If the type of something to print is jlong,
        you use
        <br>
        JLONG_FORMAT, if it's size_t, you use SIZE_FORMAT.   This change
        allows
        <br>
        macosx to compile and gets the expected result format.   I'm not
        sure
        <br>
        what the purpose of having extra %xyzzy things is.  This appears
        to be a
        <br>
        clean change.
        <br>
      </blockquote>
      <br>
      Clean except for the need to still special case _APPLE_. And I
      confess I've lost track of why that is necessary. I can see that
      because of the definition of int64_t on _APPLE_ you can't set
      JLONG_FORMAT == INT64_T_FORMAT. But I think if we have to special
      case something it should be the format specifier not the
      definition of jlong (which is what we now have). That said moving
      the "problem" from the jlong typedef to the JLONG_FORMAT
      definition isn't really any cleaner - still have an ugly _APPLE_
      check in shared code. (Though all the jlong_format_specifier()
      removal is much cleaner.)
      <br>
    </blockquote>
    The _APPLE_ check is now in 'semi' shared code,
    globalDefintions_gcc.hpp.  That file is full of Solaris, Sparc,
    Linux, APPLE, etc. specific code.  It appears to be an appropriate
    dumping ground for ugly code.<br>
    <blockquote cite="mid:50F64E60.3030807@oracle.com" type="cite">
      <br>
      If we go with Mikael's suggestion of using int64_t for jlong we
      move all the special casing to Windows. But that's because the
      compiler there does not support C99.
      <br>
    </blockquote>
    My view of the problem reported by this bug is that the typedef for
    jlong on Mac OS should be changed to match the other 64-bit
    platforms.  This fix does this.  Whether or not int64_t is a better
    definition for jlong is a different issue.  I can file an RFE to
    investigate this.<br>
    <br>
    <blockquote cite="mid:50F64E60.3030807@oracle.com" type="cite">
      <br>
      David
      <br>
      -----
      <br>
      <br>
      <blockquote type="cite">
        <br>
        Thanks,
        <br>
        Coleen
        <br>
        <br>
        <br>
        On 01/15/2013 05:57 PM, Mikael Vidstedt wrote:
        <br>
        <blockquote type="cite">
          <br>
          On 1/15/2013 11:02 AM, harold seigel wrote:
          <br>
          <blockquote type="cite">Hi,
            <br>
            <br>
            Thank you for the comments.  I think there are two remaining
            minor
            <br>
            issues.  Let me know if I missed anything.
            <br>
            <br>
            1. Use int64_t, instead of long, to define jlong?
            <br>
            <br>
                I prefer using 'long' to define 'jlong', rather than
            'int64_t',
            <br>
                because 'long' is a predefined C++ language type.  Type
            'int64_t'
            <br>
                is a Unix operating system defined type.  This would
            <br>
                unnecessarily complicate things.  For example, defining
            'jlong'
            <br>
                as 'int64_t' would require moving the definition of
            'jlong' from
            <br>
                src/cpu/x86/vm/jni_x86.h to files in the src/os_cpu/
            directories.
            <br>
            <br>
                Would it be useful to file a new bug to investigate
            using
            <br>
                'int64_t' to define 'jlong' ?
            <br>
            <br>
          </blockquote>
          <br>
          int64_t is part of the c99 standard, so it's not really an
          operating
          <br>
          system defined type per se, but I believe you're right in the
          sense
          <br>
          that it's not available in any of the standard header files on
          <br>
          Windows. But as I said I don't really have a problem defining
          jlong
          <br>
          based on long/long long if that's easier.
          <br>
          <br>
          I do think it'd be a useful exercise to see what it would take
          to use
          <br>
          int64_t to define jlong, but I'm fine with doing it as a
          separate project.
          <br>
          <br>
          <blockquote type="cite">2. Define 32-bit and 64-bit variants
            of JLONG_FORMAT in
            <br>
            src/os/posix/launcher/java_md.h ?
            <br>
            <br>
                 Would it be better to define JLONG_FORMAT as %lld for
            32-bit and
            <br>
                %ld for 64-bit for the posix variant, in file
            java_md.h?  I'm
            <br>
                unclear what the Windows variant of "%I64d" would be.
            <br>
            <br>
          </blockquote>
          <br>
          Maybe I'm missing something, but I'd say we should define
          jlong to be
          <br>
          the exact same (derived) type as int64_t, and JLONG_FORMAT
          should be
          <br>
          exactly the same as INT64_FORMAT/PRId64. For all the posix
          platforms I
          <br>
          think that should be trivial, and I'd even argue that the
          easiest way
          <br>
          to do it would be to use int64_t/PRId64 directly assuming all
          the
          <br>
          posix platforms we support have stdint.h/inttypes.h. For
          Windows,
          <br>
          judging from globalDefinitions_visCPP.hpp, it looks like
          "signed
          <br>
          __int64" and "%I64d" is the way to go regardless of 32/64.
          Does that
          <br>
          make sense?
          <br>
          <br>
          Cheers,
          <br>
          Mikael
          <br>
          <br>
          <blockquote type="cite">Thanks, Harold
            <br>
            <br>
            On 1/14/2013 2:10 PM, Mikael Vidstedt wrote:
            <br>
            <blockquote type="cite">On 2013-01-12 15:05, David Holmes
              wrote:
              <br>
              <blockquote type="cite">Sorry Harold I didn't see this
                before my other reply. Now I
                <br>
                understand your problem. We either have to:
                <br>
                <br>
                a) typedef long long jlong on all platforms; or
                <br>
                b) special case the typedef for jlong on Apple; or
                <br>
                c) special case the typedef for JLONG_FORMAT on Apple
                <br>
                <br>
                But even if we do (a) any platform that defines int64_t
                differently
                <br>
                to our jlong will mean we still need distinct format
                specifiers.
                <br>
                Further unless we know how int64_t is defined on a given
                platform
                <br>
                we don't know what format specifier to use (%ld or
                %lld).
                <br>
                <br>
                Do compilers that provide things like int64_t also
                define a format
                <br>
                specifier macro?
                <br>
              </blockquote>
              <br>
              It's part of the C99 standard (not sure about c++) - the
              types have
              <br>
              corresponding format specifier macros called PRI*, defined
              in
              <br>
              inttypes.h. For example PRId64, PRIu64 or PRIx64 can be
              used to
              <br>
              print the int64_t/uint64_t equivalents of %d, %u and %x
              <br>
              respectively. Our own internal format macros (SIZE_FORMAT,
              <br>
              INTPTR_FORMAT etc) are defined as derivatives of these.
              <br>
              <br>
              In general "long" tends to be a mess... :(
              <br>
              <br>
              /Mikael
              <br>
              <br>
              <blockquote type="cite">
                <br>
                David
                <br>
                -----
                <br>
                <br>
                On 12/01/2013 12:36 AM, harold seigel wrote:
                <br>
                <blockquote type="cite">Hi Vladimir,
                  <br>
                  <br>
                  Thank you for your comments.  Mac OS defines int64_t
                  as 'long long'.
                  <br>
                  So, int64_t needs a different format specifier than
                  jlong, which this
                  <br>
                  fix now defines as 'long'.  This is because, as shown
                  below, the
                  <br>
                  Mac OS
                  <br>
                  C++ compiler is picky about format specifiers for
                  values of types
                  <br>
                  'long
                  <br>
                  long' and 'long'.
                  <br>
                  <br>
                      $ gcc lld.cpp
                  <br>
                      lld.cpp: In function int main(int, char**):
                  <br>
                      lld.cpp:8: warning: format %lld expects type long
                  long int, but
                  <br>
                      argument 2 has type long int
                  <br>
                      lld.cpp:9: warning: format %ld expects type long
                  int, but
                  <br>
                  argument 2
                  <br>
                      has type int64_t
                  <br>
                  <br>
                      $ cat lld.cpp
                  <br>
                      #include &lt;stdio.h&gt;
                  <br>
                      #include &lt;stdint.h&gt;
                  <br>
                  <br>
                      int main(int argc, char * argv[]) {
                  <br>
                         long long_val = 5;
                  <br>
                         int64_t int64_val = 8;
                  <br>
                         printf("long_val: %ld\n", long_val);
                  <br>
                         printf("long_val: %lld\n", long_val); &lt;----
                  Line 8
                  <br>
                         printf("int64_val: %ld\n", int64_val); &lt;---
                  Line 9
                  <br>
                         printf("int64_val: %lld\n", int64_val);
                  <br>
                         return 0;
                  <br>
                      }
                  <br>
                  <br>
                  That is why I added JLONG_FORMAT.
                  <br>
                  <br>
                  Thanks, Harold
                  <br>
                  <br>
                  On 1/10/2013 9:46 PM, Vladimir Kozlov wrote:
                  <br>
                  <blockquote type="cite">Can we just define
                    INT64_FORMAT as platform specific and use it
                    <br>
                    instead of adding new JLONG_FORMAT?
                    <br>
                    <br>
                    Thanks,
                    <br>
                    Vladimir
                    <br>
                    <br>
                    On 1/10/13 10:39 AM, harold seigel wrote:
                    <br>
                    <blockquote type="cite">Hi,
                      <br>
                      <br>
                      Please review the following changes to fix bug
                      7102489.
                      <br>
                      <br>
                      Summary:
                      <br>
                      The definition of type jlong differed on Mac OS
                      from the other
                      <br>
                      64 bit
                      <br>
                      platforms.  This fix makes it consistent.  In
                      order to do this,
                      <br>
                      this fix
                      <br>
                      defines new macros, JLONG_FORMAT and
                      JULONG_FORMAT, for printing
                      <br>
                      and
                      <br>
                      scanning jlongs and julongs.
                      <br>
                      <br>
                      This fix also does some cleanup.  Methods
                      <br>
                      jlong_format_specifier() and
                      <br>
                      julong_format_specifer() were removed and some
                      format specifiers
                      <br>
                      were
                      <br>
                      replaced with appropriate macros.
                      <br>
                      <br>
                      Open webrev at
                      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~hseigel/bug_7102489/">http://cr.openjdk.java.net/~hseigel/bug_7102489/</a>
                      <br>
<a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Ehseigel/bug_7102489/">&lt;http://cr.openjdk.java.net/%7Ehseigel/bug_7102489/&gt;</a>
                      <br>
                      Bug link at
                      <a class="moz-txt-link-freetext" href="http://bugs.sun.com/view_bug.do?bug_id=7102489">http://bugs.sun.com/view_bug.do?bug_id=7102489</a>
                      <br>
                      <br>
                      Thank you,
                      <br>
                      Harold
                      <br>
                    </blockquote>
                  </blockquote>
                </blockquote>
              </blockquote>
              <br>
            </blockquote>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
  </body>
</html>