On Dec 18, 2015, at 7:41 PM, Kim Barrett <kim.barr...@oracle.com> wrote: > > On Dec 16, 2015, at 8:50 AM, Magnus Ihse Bursie > <magnus.ihse.bur...@oracle.com> wrote: >> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549 >> WebRev: >> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01 >> >> /Magnus > > I only looked at hotspot files. > > […] > There are a couple of "TBD"s below that I need to spend more time on. >
Back from holiday, and here’s my comments on those two TBDs ------------------------------------------------------------------------------ hotspot/agent/src/share/native/sadis.c 96 return (int)strlen(buf); The only call to the (file-scoped) getLastErrorString function is Java_sun_jvm_hotspot_asm_Disassembler_load_1library, which ignores the result. It would be better to change the return type of getLastErrorString to void and update the return statements accordingly. ------------------------------------------------------------------------------ hotspot/src/share/vm/adlc/arena.hpp 74 // Usual (non-placement) deallocation function to allow placement delete use size_t 75 // See 3.7.4.2 [basic.stc.dynamic.deallocation] paragraph 2. 76 void operator delete(void* p); and associated code in the .cpp file. I think this is another C++11 change: http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#429 I think the proposed code change is OK, although the comment is problematic: [basic.stc.dynamic.deallocation] is C++03 3.7.3.2 and C++11 3.7.4.2. Also, the standard change that leads to this code change is in C++11 5.3.4 [expr.new] paragraph 20 (C++02 p 19). Also, I *think* with the addition of the one-arg operator delete we don't actually use the two-arg form. If so, then I suggest removing it and the proposed new comment for the one-arg form. ------------------------------------------------------------------------------
