[9] RFR 8074840: Resolve disabled warnings for libjli and libjli_static

[David Holmes]

Hi Mikael,

On 21/03/2015 4:02 AM, Mikael Vidstedt wrote:
>
> Please review the following change which fixes a number of native
> compilation warnings in files related to libjli.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8074840
> Webrev:
> http://cr.openjdk.java.net/~mikael/webrevs/8074840/webrev.01/webrev/

Mostly looks okay, but I find some of these odd. For example:

src/java.base/share/native/libjli/java.c

if (threadStackSize < STACK_SIZE_MINIMUM)

becomes:

  if (threadStackSize < (jlong)STACK_SIZE_MINIMUM) {

but integer promotion would make that cast implicitly. And why should 
the comparison cause a warning when the subsequent assignment does not? 
What was the actual warning and on what compiler?

---

I second Kumar's query about the ergo_i586.c changes. Seems to me that 
if this is causing a problem on non Solaris/Linux then the problem is in 
the makefile (jdk/lib/CoreLibraries.gmk) because this file should be 
being excluded on platforms it is not used.

---

src/java.base/windows/native/libjli/cmdtoargs.c

Surely charLength should be a ptrdiff_t ?

---

src/java.base/windows/native/libjli/java_md.c

Aside: I find it quite bizarre that the vsnprintf family of methods take 
a size_t as the count but return an int, when on success it has to 
return the number of characters written!

---

Thanks,
David


> I built the code across all the OracleJDK platforms and there were no
> warnings on any of the platforms (in the files related to this change).
> I'm taking suggestions on specific tests to run.
>
> Some specifics:
>
> Unfortunately there is no intrinsic for cpuid in Solaris Studio, so I
> had to keep the inline asm code and the corresponding flag to disable
> the related warning (E_ASM_DISABLES_OPTIMIZATION). The alternative would
> be to move it out into a dedicated assembly file, but that seems like
> more trouble than it's worth given that the warning is harmless.
>
> I'm not entirely happy with the unsigned cast in parse_manifest.c:196,
> but unfortunately the windows prototype for read() takes an unsigned as
> its last argument, where All(tm) other platforms take a size_t. In this
> specific case 'len' will never be be more than END_MAXLEN = 0xFFFF +
> ENDHDR = 0xFFFF + 22 = 0x10015, meaning it will comfortably fit in an
> unsigned on the platforms we support. But if somebody has suggestions
> I'm all ears, doing the #ifdef dance is of course also an option.
>
> Note that the warnings were temporarily disabled as part of JDK-8074096
> [1] until such time they could be fixed the proper way. Also note that
> this change supersedes the earlier change [2] Dmitry had out for review.
> The bug [3] corresponding to that webrev will be closed as a duplicate
> of this bug (JDK-8074839).
>
> Cheers,
> Mikael
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8074096
> [2] http://cr.openjdk.java.net/~dsamersoff/JDK-8073175/webrev.01/
> [3] https://bugs.openjdk.java.net/browse/JDK-8073175
>