[systemd-devel] [PATCH] bootchart: add parameter "-C" to expand process names to the full cmdline
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Wed Mar 6 09:32:50 PST 2013
On Wed, Mar 06, 2013 at 06:02:43PM +0100, harald at redhat.com wrote:
> From: Harald Hoyer <harald at redhat.com>
>
> ---
> src/bootchart/bootchart.c | 7 ++++++-
> src/bootchart/bootchart.h | 3 ++-
> src/bootchart/log.c | 39 +++++++++++++++++++++++++++++++++++++--
> src/bootchart/svg.c | 37 +++++++++++++++++++++++++++++--------
> 4 files changed, 74 insertions(+), 12 deletions(-)
>
> diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
> index af573da..543037d 100644
> --- a/src/bootchart/bootchart.c
> +++ b/src/bootchart/bootchart.c
> @@ -76,6 +76,7 @@ bool entropy = false;
> bool initcall = true;
> bool relative = false;
> bool filter = true;
> +bool show_cmdline = false;
> bool pss = false;
> int samples;
> int len = 500; /* we record len+1 (1 start sample) */
> @@ -150,6 +151,7 @@ int main(int argc, char *argv[])
> {"output", required_argument, NULL, 'o'},
> {"init", required_argument, NULL, 'i'},
> {"no-filter", no_argument, NULL, 'F'},
> + {"cmdline", no_argument, NULL, 'C'},
> {"help", no_argument, NULL, 'h'},
> {"scale-x", required_argument, NULL, 'x'},
> {"scale-y", required_argument, NULL, 'y'},
> @@ -159,7 +161,7 @@ int main(int argc, char *argv[])
>
> gind = 0;
>
> - i = getopt_long(argc, argv, "erpf:n:o:i:Fhx:y:", opts, &gind);
> + i = getopt_long(argc, argv, "erpf:n:o:i:FChx:y:", opts, &gind);
> if (i == -1)
> break;
> switch (i) {
> @@ -175,6 +177,9 @@ int main(int argc, char *argv[])
> case 'F':
> filter = false;
> break;
> + case 'C':
> + show_cmdline = true;
> + break;
> case 'n':
> r = safe_atoi(optarg, &len);
> if (r < 0)
> diff --git a/src/bootchart/bootchart.h b/src/bootchart/bootchart.h
> index ea26c93..c42f971 100644
> --- a/src/bootchart/bootchart.h
> +++ b/src/bootchart/bootchart.h
> @@ -61,7 +61,7 @@ struct ps_struct {
> struct ps_struct *next; /* siblings */
>
> /* must match - otherwise it's a new process with same PID */
> - char name[16];
> + char name[256];
> int pid;
> int ppid;
>
> @@ -101,6 +101,7 @@ extern struct cpu_stat_struct cpustat[];
> extern int pscount;
> extern bool relative;
> extern bool filter;
> +extern bool show_cmdline;
> extern bool pss;
> extern bool entropy;
> extern bool initcall;
> diff --git a/src/bootchart/log.c b/src/bootchart/log.c
> index cf6c3a7..460adc5 100644
> --- a/src/bootchart/log.c
> +++ b/src/bootchart/log.c
> @@ -273,7 +273,25 @@ schedstat_next:
> if (!sscanf(buf, "%s %*s %*s", key))
> continue;
>
> - strncpy(ps->name, key, 16);
> + strncpy(ps->name, key, 256);
> +
Shouldn't those two identical blocks of code be a function?
Also, why not use _cleanup_close_ for the fd?
> + /* cmdline */
> + if (show_cmdline) {
> + sprintf(filename, "%d/cmdline", pid);
> + fd = openat(procfd, filename, O_RDONLY);
> + if (fd >= 0) {
> + n = read(fd, ps->name, 255);
> + if (n > 0) {
> + int i;
> + for(i=0; i < n; i++)
> + if (ps->name[i] == '\0')
> + ps->name[i] = ' ';
> + ps->name[n] = '\0';
> + }
> + close(fd);
> + }
> + }
> +
> /* discard line 2 */
> m = bufgetline(buf);
> if (!m)
> @@ -433,7 +451,24 @@ catch_rename:
> if (!sscanf(buf, "%s %*s %*s", key))
> continue;
>
> - strncpy(ps->name, key, 16);
> + strncpy(ps->name, key, 256);
> +
> + /* cmdline */
> + if (show_cmdline) {
> + sprintf(filename, "%d/cmdline", pid);
> + fd = openat(procfd, filename, O_RDONLY);
> + if (fd >= 0) {
> + n = read(fd, ps->name, 255);
> + if (n > 0) {
> + int i;
> + for(i=0; i < n; i++)
> + if (ps->name[i] == '\0')
> + ps->name[i] = ' ';
> + ps->name[n] = '\0';
> + }
> + close(fd);
> + }
> + }
> }
> }
> }
> diff --git a/src/bootchart/svg.c b/src/bootchart/svg.c
> index f8a3776..1df4263 100644
> --- a/src/bootchart/svg.c
> +++ b/src/bootchart/svg.c
> @@ -369,7 +369,7 @@ static void svg_pss_graph(void)
> top = bottom + ps->sample[i].pss;
> /* draw a label with the process / PID */
> if ((i == 1) || (ps->sample[i - 1].pss <= (100 * scale_y)))
> - svg(" <text x=\"%.03f\" y=\"%.03f\">%s [%i]</text>\n",
> + svg(" <text x=\"%.03f\" y=\"%.03f\"><![CDATA[%s]]> [%i]</text>\n",
> time_to_graph(sampletime[i] - graph_start),
> kb_to_graph(1000000.0 - bottom - ((top - bottom) / 2)),
> ps->name,
> @@ -383,10 +383,19 @@ static void svg_pss_graph(void)
> svg("\n\n<!-- PSS map - csv format -->\n");
> ps = ps_first;
> while (ps->next_ps) {
> + char *enc_name, *p;
> ps = ps->next_ps;
> if (!ps)
> continue;
> - svg("<!-- %s [%d] pss=", ps->name, ps->pid);
> + enc_name = strdup(ps->name);
> + if(!enc_name)
> + continue;
> + for (p = enc_name; *p; p++)
> + if (p[0] == '-' && p[1] == '-')
> + p[1]='_';
> +
> + svg("<!-- %s [%d] pss=", enc_name, ps->pid);
> + free(enc_name);
> for (i = 0; i < samples ; i++) {
> svg("%d," , ps->sample[i].pss);
> }
> @@ -822,9 +831,21 @@ static void svg_ps_bars(void)
> if (!ps)
> continue;
>
> - /* leave some trace of what we actually filtered etc. */
> - svg("<!-- %s [%i] ppid=%i runtime=%.03fs -->\n", ps->name, ps->pid,
> - ps->ppid, ps->total);
> + {
> + char *enc_name, *p;
> + enc_name = strdup(ps->name);
> + if(!enc_name)
> + continue;
> + for (p = enc_name; *p; p++)
> + if (p[0] == '-' && p[1] == '-')
> + p[1]='_';
> +
>From the other mail:
"""
this is needed, because we can't have "--" inside <!-- comment -->
"""
Why not put it here in a comment?
Zbyszek
More information about the systemd-devel
mailing list