This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 12/12] btrace: Store function segments as objects.
On 2017-05-09 02:55, Tim Wiederhake wrote:
2017-05-09 Tim Wiederhake <tim.wiederhake@intel.com>
gdb/ChangeLog:
* btrace.c:
* btrace.h:
* record-btrace.c:
IWBN if you could be more explicit :).
---
gdb/btrace.c | 94
++++++++++++++++++++++++++---------------------------
gdb/btrace.h | 7 ++--
gdb/record-btrace.c | 10 +++---
3 files changed, 56 insertions(+), 55 deletions(-)
diff --git a/gdb/btrace.c b/gdb/btrace.c
index 7b82000..4c7020d 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -155,14 +155,27 @@ ftrace_call_num_insn (const struct
btrace_function* bfun)
/* Return the function segment with the given NUMBER or NULL if no
such segment
exists. BTINFO is the branch trace information for the current
thread. */
-static struct btrace_function *
+const static struct btrace_function *
It would make more sense to put the "static" first.
ftrace_find_call_by_number (const struct btrace_thread_info *btinfo,
unsigned int number)
{
if (number == 0 || number > btinfo->functions.size ())
return NULL;
- return btinfo->functions[number - 1];
+ return &btinfo->functions[number - 1];
+}
+
+/* Return the function segment with the given NUMBER or NULL if no
such segment
+ exists. BTINFO is the branch trace information for the current
thread. */
It took me a surprisingly high amount of seconds to understand that this
was a const version of the function below. To avoid reapeating the
comment and to make it clear it's the same thing, you can replace the
comment of the const version to something like:
/* A const version of the function above. */
+
+static struct btrace_function *
+ftrace_find_call_by_number (struct btrace_thread_info *btinfo,
+ unsigned int number)
+{
+ if (number == 0 || number > btinfo->functions.size ())
+ return NULL;
+
+ return &btinfo->functions[number - 1];
}
/* Return non-zero if BFUN does not match MFUN and FUN,
@@ -214,37 +227,33 @@ ftrace_function_switched (const struct
btrace_function *bfun,
/* Allocate and initialize a new branch trace function segment at the
end of
the trace.
BTINFO is the branch trace information for the current thread.
- MFUN and FUN are the symbol information we have for this function.
*/
+ MFUN and FUN are the symbol information we have for this function.
+ This invalidates all struct btrace_function pointer currently held.
*/
static struct btrace_function *
ftrace_new_function (struct btrace_thread_info *btinfo,
struct minimal_symbol *mfun,
struct symbol *fun)
{
- struct btrace_function *bfun;
-
- bfun = XCNEW (struct btrace_function);
-
- bfun->msym = mfun;
- bfun->sym = fun;
+ struct btrace_function bfun {mfun, fun, 0, 0, 0, NULL, 0, 0, 0, 0,
0};
I think it would be much better to add a simple constructor to
btrace_function. For the fields that should simply be zero'ed, you can
initialize fields directly, like we do in many other places (e.g. class
inferior).
if (btinfo->functions.empty ())
{
/* Start counting at one. */
- bfun->number = 1;
- bfun->insn_offset = 1;
+ bfun.number = 1;
+ bfun.insn_offset = 1;
}
else
{
- struct btrace_function *prev = btinfo->functions.back ();
+ struct btrace_function *prev = &btinfo->functions.back ();
- bfun->number = prev->number + 1;
- bfun->insn_offset = prev->insn_offset + ftrace_call_num_insn
(prev);
- bfun->level = prev->level;
+ bfun.number = prev->number + 1;
+ bfun.insn_offset = prev->insn_offset + ftrace_call_num_insn
(prev);
+ bfun.level = prev->level;
}
- btinfo->functions.push_back (bfun);
- return bfun;
+ btinfo->functions.push_back (std::move (bfun));
+ return &btinfo->functions.back ();
I could be mistaken, but I don't think the move is very useful here,
since all fields of btrace_function are trivial (?). You could use
emplace_back instead:
btinfo->functions.emplace_back (mfun, fun);
btrace_function &bfun = btinfo->functions.back ();
...
return &bfun;
or
unsigned int number, insn_offset;
unsigned int insn_offset = prev->insn_offset + ftrace_call_num_insn
(prev);
int level = prev->level;
if (btinfo->functions.empty ())
{
/* Start counting at one. */
number = 1;
insn_offset = 1;
level = 0;
}
else
{
struct btrace_function *prev = &btinfo->functions.back ();
number = prev->number + 1;
insn_offset = prev->insn_offset + ftrace_call_num_insn (prev);
level = prev->level;
}
btinfo->functions.emplace_back (mfun, fun, number, insn_offset,
level);
return &btinfo->functions.back ();
Thanks,
Simon