I've incorporated the suggestions into the v2/soon-v3 patchset, just a couple
things worth noting on this v1 patchset for the record.
On Fri, May 29, 2026, at 3:06 PM, Aaron Merey wrote:
>> + { "gmon", 'g', NULL, 0, N_("Generate gmon.BUILDID.out files for each
>> binary."), 0 },
>> + { "hist-split",'G', HIST_SPLIT_OPTS, 0, N_("Histogram splitting method
>> for gmon, default 'even'."), 0 },
>
> It would be helpful to very briefly describe the difference between
> even and flex here.
Space is limited, so I went for "Split gmon histogram output into even or
flexible chunks, default 'even'."
>> + case 'p':
>> + pid = atoi(arg);
>
> We should have some error checking that pid > 0 in case arg isn't a valid pid.
ack, added a check patterned on the existing code in eu-stack.
>> + err = this->find_procfile (dwfl, &pid, &elf, &elf_fd);
>> + if (err < 0)
>
> find_procfile returns positive errnos when it encounters an error.
> So this error handler will never run and elf/elf_fd remain NULL/-1.
Good catch, this also led to a related bugfix in find_procfile().
Which is the main thing I wanted to document in this email.
I found the getline(3) man page was completely ambiguous
as to whether the string is terminated with '\n'
(it says a newline is included "if it found one"),
so the code had to be updated to handle either case:
long val = strtol (&line.c_str()[5], &endptr, 10);
if ((errno == ERANGE && val == LONG_MAX)
|| (*endptr != 0 && *endptr != '\n')
/* <- getline(3) ambiguous on what ends the string */
|| val < 0 || val != (pid_t) val)
*pid = 0;
else
*pid = (pid_t) val;
>> +#if 0
>> + Dwarf_Addr bias;
>> + Elf *elf = dwfl_module_getelf (mod, &bias);
>> + (void)elf;
>> +#endif
>
> Similar comment as the previous #if 0 block. If this is not intended for
> debug builds going forward, let's get rid of it. Otherwise let's use a macro
> defined near the top of the file to control whether it's enabled. Both of the
> existing #if 0 blocks could be controlled with one macro if appropriate.
ack, have opted to guard the code with a STACKPROF_STATS_DEBUG macro
--
All the best,
Serhei
http://serhei.io
