Hello Yasumasa and Chris, thanks for your input . Here is a new webrev , without the unneeded memset-calls after calloc .
http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.1/ Hope everyone is happy with this now 😉 ! Best regards, Matthias > Hi Matthias, > > src/jdk.hotspot.agent/linux/native/libsaproc/symtab.c: > ``` > 405 // guarantee(symtab == NULL, "multiple symtab"); > 406 symtab = (struct symtab*)calloc(1, sizeof(struct symtab)); > 407 if (symtab == NULL) { > 408 goto quit; > 409 } > 410 memset(symtab, 0, sizeof(struct symtab)); > ``` > > Why do you call memset() to clear symtab in L410? > symtab is allocated via calloc() in L406, so symtab would already cleared. > > > Thanks, > > Yasumasa (ysuenaga) > > > On 2019/09/03 18:14, David Holmes wrote: > > Hi Matthias, > > > > Re-directing to serviceability-dev. > > > > David > > > > On 3/09/2019 5:42 pm, Baesken, Matthias wrote: > >> Hello, please review the following small fix . > >> > >> In jdk.hotspot.agent native code (linux / macosx) we miss to check the > result of malloc/calloc a few times . > >> This should be adjusted. > >> Additionally I added initialization to the symtab array in symtab.c > >> (by > calling memset to make sure we have a defined state ) . > >> > >> > >> > >> One question (was not really sure about this one so I did not change it so > far) : > >> > >> > http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.0/src/jdk.hotspot. > agent/macosx/native/libsaproc/symtab.c.frames.html > >> > >> 359 void destroy_symtab(symtab_t* symtab) { > >> 360 if (!symtab) return; > >> 361 free(symtab->strs); > >> 362 free(symtab->symbols); > >> 363 free(symtab); > >> 364 } > >> > >> > >> > >> Here we miss to close symtab->hash_table (opened by dbopen) , is it > needed (haven't used dbopen much - maybe someone can comment on > this)? > >> > >> > >> bug/webrev : > >> > >> https://bugs.openjdk.java.net/browse/JDK-8230466 > >> > >> http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.0/ > >> > >> > >> Thanks and best regards, Matthias > >>
