MDEV-38753: Debugging help: print which MEM_ROOT the object is on#4615
MDEV-38753: Debugging help: print which MEM_ROOT the object is on#4615
Conversation
spetrunia
force-pushed
the
10.6-MDEV-38753
branch
from
f7b8fe9 to
637ad36
Compare
|
How |
spetrunia
force-pushed
the
10.6-MDEV-38753
branch
from
637ad36 to
c0781c0
Compare
DaveGosselin-MariaDB
left a comment
DaveGosselin-MariaDB
left a comment
There was a problem hiding this comment.
Mostly cosmetic but a couple of nontrivial suggestions.
| #ifndef DBUG_OFF | ||
|
|
||
| /* Check if ptr points to memory on the mem_root */ | ||
|
|
There was a problem hiding this comment.
Consider removing this blank line to have the function immediately follow the comment (consistent with dbug_which_mem_root below)
There was a problem hiding this comment.
I thought most functions actually do the reverse - have a blank line between the comment and function.
Do I change it in dbug_which_mem_root instead?
sql/sql_test.cc
Outdated
|
|
||
| bool dbug_is_mem_on_mem_root(const MEM_ROOT *mem_root, void *ptr) | ||
| { | ||
| const USED_MEM *ptrs[]={mem_root->free, mem_root->used}; |
There was a problem hiding this comment.
As I understand, there can be no MEM_ROOT covering address 0x0, so consider adding a shortcut here when ptr is NULL:
if (!ptr)
return false;
There was a problem hiding this comment.
No MEM_ROOT covers the NULL pointer, but the current code already handles it:
(gdb) p dbug_is_mem_on_mem_root(thd->mem_root, 0)
$1 = false
We are in debugger, performance is not a concern... why add shortcuts?
| for (const USED_MEM *block= *p; block; block=block->next) | ||
| { | ||
| const char *start= (const char*)block; | ||
| const char *end= start + block->size - block->left; |
There was a problem hiding this comment.
It seems to me that a valid block's size should never be zero (or less), but I wouldn't be surprised if a zero-length block is possible for some special case. Consider asserting that it is greater-than zero here. (similar thoughts apply to block->left)
In any case, there's an implicit assumption here that end > start so we should avoid ambiguity in the case that somehow end <= start is possible.
There was a problem hiding this comment.
Added an assert that end >= start.
Add two functions intended for use from debugger: bool dbug_is_mem_on_mem_root(MEM_ROOT *mem_root, void *ptr); const char *dbug_which_mem_root(THD *thd, void *ptr); Also, collect declarations of all other functions intended for use from debugger in sql/sql_test.h
spetrunia
force-pushed
the
10.6-MDEV-38753
branch
from
d5ae462 to
06322f5
Compare
4 participants
Add two functions intended for use from debugger:
bool dbug_is_mem_on_mem_root(MEM_ROOT *mem_root, void *ptr);
const char *dbug_which_mem_root(THD *thd, void *ptr);
Also, collect declarations of all other functions intended for use from debugger in sql/sql_debug.h