Request for review: 7102489: RFE: cleanup jlong typedef on __APPLE__ and _LP64 systems

[harold seigel]

Hi David,

Thanks for your comments.  My replies are interspersed.

Harold

On 1/11/2013 2:33 AM, David Holmes wrote:
> Hi Harold,
>
> On 11/01/2013 4:39 AM, harold seigel wrote:
>> Hi,
>>
>> Please review the following changes to fix bug 7102489.
>>
>> Summary:
>> The definition of type jlong differed on Mac OS from the other 64 bit
>> platforms. This fix makes it consistent. In order to do this, this fix
>> defines new macros, JLONG_FORMAT and JULONG_FORMAT, for printing and
>> scanning jlongs and julongs.
>>
>> This fix also does some cleanup. Methods jlong_format_specifier() and
>> julong_format_specifer() were removed and some format specifiers were
>> replaced with appropriate macros.
>
> General clean up looks good. As long as we have jlong and int64 types 
> in use I much prefer to JLONG_FORMAT and INT64_FORMAT in the sources.
>
> I still wonder how those os:: functions came about in the first place :)

Coleen looked into this but they date from long ago.
>
> A few comments ...
>
> src/os/posix/launcher/java_md.h
>
>   67 #define JLONG_FORMAT "%lld"
>
> Doesn't this need to have both 32-bit and 64-bit variants?
>
> -- 
>
> src/os/windows/launcher/java_md.h
>
> + #define JLONG_FORMAT "%I64d"
>
> Ditto ?

These changes just replace calls to jlong_format_specifier() with a 
JLONG_FORMAT macro that has the same value.  Since there wasn't a need 
for 32-bit and 64-bit variants for jlong_format_specifier(), I don't 
think variants were needed for JLONG_FORMAT.

> -- 
>
> src/share/vm/utilities/globalDefinitions_gcc.hpp
>
> Why do we need a special case for Apple? AFAIK for ILP32 and LP64 
> "long long" is always 64-bits so %lld should always be valid.
>
This answer is similar to the one I sent to Vladimir.  We need a special 
case for Apple because its compiler is picky about format specifiers.  
It will issue a warning for values of type long that have format 
specifiers of %lld.  For example, on Mac OS, this program gets a 
compilation warning at line 6, but not line 5.

$ gcc lld.cpp
lld.cpp: In function int main(int, char**):
lld.cpp:6: warning: format %lld expects type long long int, but argument 
2 has type long int

$ cat lld.cpp
#include <stdio.h>

int main(int argc, char * argv[]) {
   long val = 5;
   printf("val: %ld\n", val);
   printf("val: %lld\n", val); <--- line 6
   return 0;
}
> -- 
>
> src/share/vm/utilities/ostream.cpp
>
> I think the N.B comments could be deleted.

I agree.  I'll delete those comments.

>
> -- 
>
> Thanks,
> David
>
>
>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_7102489/
>> <http://cr.openjdk.java.net/%7Ehseigel/bug_7102489/>
>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=7102489
>>
>> Thank you,
>> Harold