* [PATCH] zprof: Don't tally all anonymous functions as though they were a single function named "(anon)".
@ 2020-05-26 22:06 Daniel Shahaf
2020-05-26 22:15 ` Daniel Shahaf
0 siblings, 1 reply; 2+ messages in thread
From: Daniel Shahaf @ 2020-05-26 22:06 UTC (permalink / raw)
To: zsh-workers
Before:
% zmodload zsh/zprof
% () :
% () :
% zprof
num calls time self name
-----------------------------------------------------------------------------------
1) 2 0.08 0.04 100.00% 0.08 0.04 100.00% (anon)
After:
% zmodload zsh/zprof
% () :
% () :
% zprof
num calls time self name
-----------------------------------------------------------------------------------
1) 1 0.04 0.04 50.45% 0.04 0.04 50.45% (anon) [:3]
2) 1 0.04 0.04 49.55% 0.04 0.04 49.55% (anon) [:2]
---
Should «funcstack» be checked for NULL before it's dereferenced?
The funcstack frame is added in doshfunc() while wrappers are called
from runshfunc(). I don't see any case in which doshfunc() won't have
been called by the time runshfunc() has, but then I don't completely
grok the sense of the EF_RUN flag as used in doshfunc().
Cheers,
Daniel
Src/Modules/zprof.c | 31 ++++++++++++++++++++++++++++---
Src/exec.c | 20 ++++++++++++++++++--
2 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/Src/Modules/zprof.c b/Src/Modules/zprof.c
index bc97771c0..2d8ed4f68 100644
--- a/Src/Modules/zprof.c
+++ b/Src/Modules/zprof.c
@@ -213,7 +213,25 @@ bin_zprof(UNUSED(char *nam), UNUSED(char **args), Options ops, UNUSED(int func))
return 0;
}
-/**/
+static char *
+name_for_anonymous_function(char *name)
+{
+ char lineno[DIGBUFSIZE];
+ char *parts[7];
+
+ convbase(lineno, funcstack[0].flineno, 10);
+
+ parts[0] = name;
+ parts[1] = " [";
+ parts[2] = funcstack[0].filename ? funcstack[0].filename : "";
+ parts[3] = ":";
+ parts[4] = lineno;
+ parts[5] = "]";
+ parts[6] = NULL;
+
+ return sepjoin(parts, "", 0);
+}
+
static int
zprof_wrapper(Eprog prog, FuncWrap w, char *name)
{
@@ -224,12 +242,19 @@ zprof_wrapper(Eprog prog, FuncWrap w, char *name)
struct timeval tv;
struct timezone dummy;
double prev = 0, now;
+ char *name_for_lookups;
+
+ if (is_anonymous_function_name(name)) {
+ name_for_lookups = name_for_anonymous_function(name);
+ } else {
+ name_for_lookups = ztrdup(name);
+ }
if (zprof_module && !(zprof_module->node.flags & MOD_UNLOAD)) {
active = 1;
- if (!(f = findpfunc(name))) {
+ if (!(f = findpfunc(name_for_lookups))) {
f = (Pfunc) zalloc(sizeof(*f));
- f->name = ztrdup(name);
+ f->name = name_for_lookups;
f->calls = 0;
f->time = f->self = 0.0;
f->next = calls;
diff --git a/Src/exec.c b/Src/exec.c
index 2b8e2167f..29f4fc5ca 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5147,10 +5147,26 @@ exectime(Estate state, UNUSED(int do_exec))
return lastval;
}
-/* Define a shell function */
-
+/* The string displayed in lieu of the name of an anonymous function (in PS4,
+ * zprof output, etc)
+ */
static const char *const ANONYMOUS_FUNCTION_NAME = "(anon)";
+/*
+ * Take a function name argument and return true iff it is equal to the string
+ * used for the names of anonymous functions, "(anon)".
+ *
+ * Note that it's possible to define a named function literally called "(anon)"
+ * (though I doubt anyone would ever do that).
+ */
+/**/
+int is_anonymous_function_name(const char *name)
+{
+ return !strcmp(name, ANONYMOUS_FUNCTION_NAME);
+}
+
+/* Define a shell function */
+
/**/
static int
execfuncdef(Estate state, Eprog redir_prog)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] zprof: Don't tally all anonymous functions as though they were a single function named "(anon)".
2020-05-26 22:06 [PATCH] zprof: Don't tally all anonymous functions as though they were a single function named "(anon)" Daniel Shahaf
@ 2020-05-26 22:15 ` Daniel Shahaf
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Shahaf @ 2020-05-26 22:15 UTC (permalink / raw)
To: zsh-workers
Daniel Shahaf wrote on Tue, 26 May 2020 22:06 +00:00:
> +++ b/Src/Modules/zprof.c
> @@ -213,7 +213,25 @@ bin_zprof(UNUSED(char *nam), UNUSED(char **args),
> @@ -224,12 +242,19 @@ zprof_wrapper(Eprog prog, FuncWrap w, char *name)
> + char *name_for_lookups;
> +
> + if (is_anonymous_function_name(name)) {
> + name_for_lookups = name_for_anonymous_function(name);
> + } else {
> + name_for_lookups = ztrdup(name);
Memory leak when either of the next two conditions is false.
> + }
>
> if (zprof_module && !(zprof_module->node.flags & MOD_UNLOAD)) {
> active = 1;
> - if (!(f = findpfunc(name))) {
> + if (!(f = findpfunc(name_for_lookups))) {
> f = (Pfunc) zalloc(sizeof(*f));
> - f->name = ztrdup(name);
> + f->name = name_for_lookups;
> f->calls = 0;
> f->time = f->self = 0.0;
> f->next = calls;
> Should «funcstack» be checked for NULL before it's dereferenced?
> The funcstack frame is added in doshfunc() while wrappers are called
> from runshfunc(). I don't see any case in which doshfunc() won't have
> been called by the time runshfunc() has, but then I don't completely
> grok the sense of the EF_RUN flag as used in doshfunc().
This question stands.
Cheers,
Daniel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-26 22:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 22:06 [PATCH] zprof: Don't tally all anonymous functions as though they were a single function named "(anon)" Daniel Shahaf
2020-05-26 22:15 ` Daniel Shahaf
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).