Hi Matthias,
On 9/5/19 01:19, Baesken, Matthias wrote:
Hello Serguei, thanks for the comments .
http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.1/src/jdk.hotspot.
agent/linux/native/libsaproc/symtab.c.frames.html
279 build_id_to_debug_filename (size_t size, unsigned char *data)
280 {
. . .
283 filename = malloc(strlen (debug_file_directory) + (sizeof
"/.build-id/" - 1) + 1
284 + 2 * size + (sizeof ".debug" - 1) + 1);
285 if (filename == NULL) {
286 return NULL;
287 }
. . .
312 char *filename
313 = (build_id_to_debug_filename (note->n_descsz, bytes));
314 if (filename == NULL) {
315 return NULL;
316 }
There is no need to check filename for NULL at the line 314 as the function
build_id_to_debug_filename with new check at the line 285 never returns
NULL.
At line 286 of build_id_to_debug_filename we now return NULL (in
case malloc cannot alloc memory). So we should check this also
at line 314 , or do I miss something ?
Oh, sorry.
You are right, thanks!
http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.1/src/jdk.hotspot.
agent/macosx/native/libsaproc/MacosxDebuggerLocal.m.frames.html
354 array = (*env)->NewByteArray(env, numBytes);
. . .
376 if (pages == NULL) {
377 return NULL;
378 }
379 mapped = calloc(pageCount, sizeof(int));
380 if (mapped == NULL) {
381 free(pages);
382 return NULL;
383 }
Just a question:
We do not release the array allocated at line 354 because this local
reference
will be auto-released when returning to Java. Is this correct?
Good point, I think we better add DeleteLocalRef here in case of the new
early returns .
http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.1/src/jdk.hotspot.
agent/macosx/native/libsaproc/symtab.c.frames.html
69 if (is_debug()) {
70 DBT rkey, rvalue; 71 char* tmp = (char
*)malloc(strlen(symtab->symbols[i].name) + 1);
72 if (tmp != NULL) {
73 strcpy(tmp, symtab->symbols[i].name);
74 rkey.data = tmp;
75 rkey.size = strlen(tmp) + 1;
76 (*symtab->hash_table->get)(symtab->hash_table, &rkey, &rvalue, 0);
77 // we may get a copy back so compare contents
78 symtab_symbol *res = (symtab_symbol *)rvalue.data;
79 if (strcmp(res->name, symtab->symbols[i].name) ||
80 res->offset != symtab->symbols[i].offset ||
81 res->size != symtab->symbols[i].size) {
82 print_debug("error to get hash_table value!\n");
83 }
84 free(tmp);
85 }
If malloc returns NULL then this debugging part will be we silently skipped.
In other such cases there is an attempt to print a debug message.
For instance:
140 symtab = (symtab_t *)malloc(sizeof(symtab_t));
141 if (symtab == NULL) {
142 print_debug("out of memory: allocating symtab\n");
143 return NULL;
144 }
I understand that print_debug can fail with out of memory as well.
But it depends on its implementation.
Thanks,
Serguei
That's a good idea . I added a message .
See new webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8230466.2/
Thank you for the update!
It looks good to me.
Thanks,
Serguei
Thanks, Matthias
