-
-
Notifications
You must be signed in to change notification settings - Fork 2k
MDEV-38753: Debugging help: print which MEM_ROOT the object is on #4615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 10.6
Are you sure you want to change the base?
Conversation
f7b8fe9 to
637ad36
Compare
|
How |
637ad36 to
c0781c0
Compare
DaveGosselin-MariaDB
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whichever way, only be consistent.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's your choice, I said only "consider" 😊
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
d5ae462 to
06322f5
Compare
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