New webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.3/ Best regards, Matthias > IMO the solution with goto makes it even worse. > If you don't want to introduce the wrapper, could you please restore > changes in LinuxDebuggerLocal_attach0 from webrev.1 > > --alex > > On 02/21/2020 00:32, Baesken, Matthias wrote: > > Hi Alex , > > > > new webrev : > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.2/ > > > > Best Regards, Matthias > > > > > >> > >> Hi Matthias, > >> > >> Looks good in general, but I think it makes sense to fix #2 cases (at > >> least I see them in LinuxDebuggerLocal). If GetStringUTFChars fails, the > >> code will crash. > >> Also I see GetStringUTFChars(str, JNI_FALSE). This look bad as well - > >> 2nd arg is a pointer, so it should be NULL or nullptr. > >> > >> As for #1 and #3 - AFAIU they are both right ways. > >> If GetStringUTFChars fails, it throws OOM and return NULL. > >> > >> And one more thing to consider. > >> LinuxDebuggerLocal_attach0 function looks terrible - 7 > >> ReleaseStringUTFChars calls for 2 GetStringUTFChars. > >> Maybe it make sense to introduce simple wrapper like AutoJavaString in > >> src/jdk.hotspot.agent/windows/native/libsaproc/sawindbg.cpp > >> It would make the code simpler and less error prone. > >> > >> --alex > >> > >
