Trace checker: check ptr values #51
Conversation
anast2024
force-pushed
the
pointer-val
branch
4 times, most recently
from
9918d5e to
f1385d4
Compare
Signed-off-by: Anastasia Basho <[email protected]>
anast2024
force-pushed
the
pointer-val
branch
from
db49f64 to
10eba8a
Compare
| @@ -1,3 +1,4 @@ | |||
| #include "coldtrace/entries.h" | |||
| #define PTR_MASK 0xFFFFFFFFFFFF0000UL | ||
| #define ZERO_FLAG 0x80 | ||
|
|
||
| #define SHIFT_VALUE 16 |
There was a problem hiding this comment.
can we change to PTR_SHIFT_VALUE and put it with PTR_MASK so it's clear that they belong together?
| log_info("register expected trace thread=%lu", tid); | ||
| _expected[tid] = trace; | ||
| _expected[tid] = trace; | ||
| size_t array_size = sizeof(entry_ptr) / sizeof(uint64_t); |
There was a problem hiding this comment.
let's use MAX_ENTRY_VALUES
| typedef void (*entry_callback)(const void *entry); | ||
| static size_t _entry_callback_count = 0; | ||
| static entry_callback _entry_callbacks[MAX_ENTRY_CALLBACKS]; | ||
| static uint64_t entry_ptr[MAX_ENTRY_VALUES]; |
There was a problem hiding this comment.
let's stick to the same convention as the variables above and start this with _ as it is static as well
There was a problem hiding this comment.
maybe _entry_ptr_values to make more clear what it is?
| int i) | ||
| { | ||
| // check ptr required | ||
| if (iter->check != -1) { |
There was a problem hiding this comment.
let's make a macro for the -1 and call it something like NO_CHECK
| entry_ptr[iter->check] = type_ptr; | ||
| log_info("thread=%lu entry=%d match=%s match=ptr=%lu", tid, i, | ||
| coldtrace_entry_type_str((iter->e)->type), type_ptr); | ||
| // ptr match |
There was a problem hiding this comment.
let's put the comments inside the block that they concern
|
|
||
| for (int i = 0; iter_next(it); iter_advance(&it), i++) { | ||
| coldtrace_entry_type type = iter_type(it); | ||
| uint64_t type_ptr = iter_pointer_value(it); |
There was a problem hiding this comment.
can we also call it ptr_value or pointer_value on the left side? Let's make naming consistent
| _expected[tid] = trace; | ||
| size_t array_size = sizeof(entry_ptr) / sizeof(uint64_t); | ||
| for (size_t i = 0; i < array_size; i++) { | ||
| entry_ptr[i] = (uint64_t)-1; |
There was a problem hiding this comment.
let's make a define for this -1 to have a CHECK_NONE or CHECK_UNINITIALIZED
1 participant
No description provided.