RFR(M): [ping!] 8139116: Fixes for warning "format not a string literal"

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Mon Oct 26 17:07:31 UTC 2015


Hi Volker, 

thanks for the review!  See comments inline.

> -----Original Message-----
> From: Volker Simonis [mailto:volker.simonis at gmail.com]
> Sent: Montag, 26. Oktober 2015 17:39

> It seems that versions before libc5 on Linux did not support "*".
> Have you checked that all currently supported platforms support it now?
It's used in other places already, e.g. vm/gc/shared/taskqueue.cpp:62.

> - xmlstream.cpp
> What does the following line mean?
> 343 // This fails to compile with clang-503.0.40
If you remove the PRAGMA, that compiler issues a warning.  
I'll add "If you remove the PRAGMA, this ...." to the comment to make it more clear.

> Which version of clang is this? Do we use that on Mac/Linux (i.e. how
> relevant is it)?
It's:
    Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)  
   Target: x86_64-apple-darwin13.3.0.

> - compilationPolicy.cpp
> Update the copyright year :)
I just rebased the change, and another change updated the Copyright.
 
> Did you do some manual checks to verify that the format of the output
> didn't change (especially for heapInspection.cpp)?
Yes, this one I actually put into a small testprogram and verified ... good that
this code is gone now I think!

The new webrev:
http://cr.openjdk.java.net/~goetz/webrevs/8139116-nonliteral/webrev.02/

Best regards,
  Goetz.


> 
> Regards,
> Volker
> 
> 
> On Fri, Oct 23, 2015 at 8:41 AM, Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com> wrote:
> > Hi,
> >
> > Could some Reviewer please have a look at this change?  I also need a
> sponsor!
> > I fixed the issue reported by Dmitry and rebased the change to todays hs-
> rt:
> > http://cr.openjdk.java.net/~goetz/webrevs/8139116-
> nonliteral/webrev.01/
> >
> > Thanks and best regards,
> >   Goetz.
> >
> >
> > From: Lindenmaier, Goetz
> > Sent: Tuesday, October 13, 2015 11:32 AM
> > To: hotspot-runtime-dev at openjdk.java.net
> > Subject: RFR(M): 8139116: Fixes for warning "format not a string literal"
> >
> > Hi,
> >
> > The gcc build has -Wformat enabled.  This warns if the format string in a
> printf-like function is not a string literal, because if so it can not check that the
> types of the arguments match the format specifiers.
> >
> > In a row of places this warning is suppressed by
> PRAGMA_FORMAT_NONLITERAL_IGNORED. This change fixes all but three
> occurrences of this pragma.
> >
> > This especially improves heapInspection.hpp where a static buffer is used
> to print the format string, which is not multithreading safe.
> >
> > I didn't understand the purpose of the original code in heapDumper.cpp.
> Was this
> > just bogus code, or did I oversee something?
> >
> > Also, I don't know why there was the workaround for '*' in
> heapInspection.cpp and globals.cpp.
> > Is there any platform that does not understand this?
> >
> > Please review this change. I please need a sponsor.
> > http://cr.openjdk.java.net/~goetz/webrevs/8139116-
> nonliteral/webrev.00/
> >
> > Best regards,
> >   Goetz.


More information about the hotspot-runtime-dev mailing list