On Feb 21, 2015, at 5:38 PM, Kumar Srinivasan <kumar.x.sriniva...@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
