RFR(S): JDK-8073584 Native compilation warning in unpack code

[John Rose]

On Feb 21, 2015, at 5:38 PM, Kumar Srinivasan <kumar.x.srinivasan at oracle.com> wrote:
> 
> Hi Dmitry,
> 
> adding John on this.
> 
> unpack.cpp
> What is wrong with the  unary operator ? Does this emit a compiler warning ?

(It's the ternary operator right?)  The problem is that the format string (oh, the cleverness!!) is non-constant.

> -  sprintf(buf, ((uint)e.tag < CONSTANT_Limit)? TAG_NAME[e.tag]: "%d", e.tag);
> +  if ((uint)e.tag < CONSTANT_Limit) {
> +    sprintf(buf, "%s", TAG_NAME[e.tag]);
> +  }
> +  else {
> +    sprintf(buf, "%d", e.tag);
> +  }
> 
> If you are eliminating the unary operator then, the formatting should be
> 
> if (.....)
>   ......
> else
>   ......
> or
> if (.....) {
> ....
> else {  <--- note, please don't introduce new formatting/conventions.

I agree on that.  Let's be whitespace chameleons.

>   ....
> }
> 
> main.cpp:
> 
> +      (void) (fread(&filecrc, sizeof(filecrc), 1, u.infileptr) + 1);
> 
> UGH. What about other compilers are they ok with this ? This may very well
> get suppressed for gcc, but might emit warnings on MSVC or SunStudio.

Surely it would be better to bind the result of fread to a throwaway variable, if there's a warning about unused return value.
    void* fread_result_ignored =
         fread(&filecrc, sizeof(filecrc), 1, u.infileptr);

— John