RFR(M): 8140594: Various minor code improvements (compiler)

[Lindenmaier, Goetz]

Sorry for the double failure!
I think this really works now, Christoph verified it with VS2010, thanks!
http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.03/

I just removed prefix_len, it's added to newbuf_len, below anyways.

Best regards,
  Goetz.



> -----Original Message-----
> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> Sent: Montag, 9. Mai 2016 13:42
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot compiler
> <hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(M): 8140594: Various minor code improvements (compiler)
> 
> Hi Goetz,
> 
> On 09.05.2016 12:30, Lindenmaier, Goetz wrote:
> > Hi Tobias,
> >
> > sorry for that!  No, it's not just missing include, windows does not
> > implement snprintf before Visual Studio 2015.
> > http://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-
> 2010
> >
> > As this is only a build-time tool, I would prefer skipping this altogether
> > above adding windows specific code:
> > http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.02/
> 
> That's fine with me but unfortunately the build still fails on Windows:
> 
> c:/jprt/T/P1/105246.tohartma/s/hotspot/src/share/vm/logging/logTagSet.cp
> p(112) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of
> data
> 
> Best regards,
> Tobias
> >
> > Best regrds,
> >  Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
> >> Sent: Montag, 9. Mai 2016 11:08
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot compiler
> >> <hotspot-compiler-dev at openjdk.java.net>
> >> Subject: Re: RFR(M): 8140594: Various minor code improvements
> (compiler)
> >>
> >> Hi Goetz,
> >>
> >> the build failed on Windows with:
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(721)
> :
> >> error C3861: 'snprintf': identifier not found
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(723)
> :
> >> error C3861: 'snprintf': identifier not found
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(728)
> :
> >> error C3861: 'snprintf': identifier not found
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(730)
> :
> >> error C3861: 'snprintf': identifier not found
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(740)
> :
> >> error C3861: 'snprintf': identifier not found
> >> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(742)
> :
> >> error C3861: 'snprintf': identifier not found
> >>
> >> Looks like a missing include.
> >>
> >> Best regards,
> >> Tobias
> >>
> >> On 09.05.2016 10:40, Tobias Hartmann wrote:
> >>> Hi Goetz,
> >>>
> >>> thanks for the explanation, this looks good to me!
> >>>
> >>> I'm running the hs-comp PIT now and will push the fix afterwards, if
> there
> >> are no objections.
> >>>
> >>> Thanks,
> >>> Tobias
> >>>
> >>> On 09.05.2016 10:30, Lindenmaier, Goetz wrote:
> >>>> Hi Tobias,
> >>>>
> >>>>> I can sponsor your fix but since we are close to JDK 9 FC, we are not
> >> allowed
> >>>>> to push enhancements to hs-comp. Shouldn't this be a "bug" anyway?
> >>>> Thanks for reviewing and the offer to sponsor!
> >>>> I changed the Jira issue to bug.  Sorry for being that close to FC with my
> >> RFR.
> >>>>
> >>>>>> - size of pointer passed to jio_snprintf()
> >>>>> Please also fix the indentation in line 6004.
> >>>> Fixed.
> >>>>
> >>>>>> classLoader.cpp
> >>>>>> - jio_snprintf does null termination. But it might return -1 if
> truncated,
> >>>>>>   in this case array access at -1.
> >>>>> You can remove "int n;"
> >>>> Fixed.
> >>>>
> >>>>>> generateOopMap.cpp
> >>>>>> - Remaining fields not initialized.
> >>>>> I would put the loop bodies in a new line.
> >>>> Fixed.
> >>>>
> >>>>>> relocator.cpp
> >>>>>> - delta might be -4 ... assert returns.
> >>>>> I don't understand this change. If delta is -4, the assert in the baseline
> >> version
> >>>>> is triggered. With your fix, the assert is triggered as well.
> >>>>
> >>>> Sorry, that's a bad comment.  I meant that the assert does not
> >>>> protect against getting -4 in that line, because coverity knows
> >>>> about -XX:SuppressErrorAt.  But that's pointless, as assert goes
> >>>> away in product builds anyways.
> >>>>
> >>>> The assert shall be triggered, therefore I moved it out of the if().
> >>>> But the if() now protects against oob with -4 or smaller.
> >>>>
> >>>> I also rebased the change, and added you as reviewer.  Find the new
> >>>> Webrev here:
> >>>> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.01/
> >>>> And the incremental patch:
> >>>> http://cr.openjdk.java.net/~goetz/wr16/8140594-
> >> covCo/webrev.01/incremental_diff.patch
> >>>>
> >>>> Best regards,
> >>>>   Goetz.
> >>>>