Conversation
Signed-off-by: Gideon Sebastian Geier <gideon.sebastian.geier@huawei.com>
Signed-off-by: Gideon Sebastian Geier <gideon.sebastian.geier@huawei.com>
| char *var = getenv("COLDTRACE_PATH"); | ||
| if (var == NULL) { | ||
| log_fatal("Set COLDTRACE_PATH to a valid directory\n"); | ||
| return 0; |
There was a problem hiding this comment.
please use proper names for bool
| if (path == NULL) { | ||
| log_fatal("error: invalid path\n"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
is this really necessary? seems impossible. I think it's better to just document the assumption that this path is a valid C string
| default: | ||
| log_fatal("Unknown entry size"); | ||
| break; | ||
| } |
There was a problem hiding this comment.
this code is now reachable, which looks like it could make /test/checkers/trace_checker.c#L424 coldtrace_writer_close loop forever instead of crashing.
| { | ||
| if (!is_type_valid(type)) { | ||
| log_fatal("Invalid entry type (at %s:%d)", __FILE__, __LINE__); | ||
| return 0; |
There was a problem hiding this comment.
this only works well if this code is never reached. Otherwise the caller will take a 0-sized entry, which increases the impl->offset by 0, and the same memory will be doubly-allocated by multiple entries.
Almost every line so far has issues. I will continue the review after you have checked your MR a little carefully.
2 participants
to resolve #102