John, Generally speaking, we should select warnings carefully - not just turn all of it on.
For instance, I don't see any value of "format is not a string literal" warning for JDK code. Please, see also below. On 2015-02-22 06:20, John Rose wrote: > 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. Yes. Actually sprintf here could be replaced with just a strcpy. >> - 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. I'll change it. > >> .... >> } >> >> 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. It works for all JDK compilers. > > 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); It causes "assigned but unused variable" warning. -Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.