zsh-workers
 help / color / mirror / code / Atom feed
* [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).