Re: [PATCH] perf: fix uninitialized variable in thread__set_comm()

[Stephane Eranian]

Arnaldo,

Did you also integrate this patch?


On Tue, Sep 6, 2011 at 5:00 PM, Stephane Eranian <eranian@xxxxxxxxxx> wrote:
>
> In thread__set_comm(), a command string could be freed and then
> reused to point to a new command. Yet, the length was never reset
> nor recomputed. This could lead to bogus column formatting.
>
> This patch below fixes the problem by having thread__set_comm()
> immediately compute the string length in thread->comm_len. It
> gets rid of thread__comm_len() which is not needed anymore.
> It was doing the lazy eval of strlen() which could cause the
> issue.
>
> Signed-off-by: Stephane Eranian <eranian@xxxxxxxxxx>
> ---
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 677e1da..b883821 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -61,7 +61,7 @@ static void hists__calc_col_len(struct hists *self, struct hist_entry *h)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unresolved_col_width);
> Â Â Â Â}
>
> - Â Â Â len = thread__comm_len(h->thread);
> + Â Â Â len = h->thread->comm_len;
> Â Â Â Âif (hists__new_col_len(self, HISTC_COMM, len))
> Â Â Â Â Â Â Â Âhists__set_col_len(self, HISTC_THREAD, len + 6);
>
> @@ -731,7 +731,7 @@ static size_t hist_entry__fprintf_callchain(struct hist_entry *self,
> Â Â Â Â Â Â Â Âstruct sort_entry *se = list_first_entry(&hist_entry__sort_list,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â typeof(*se), list);
> Â Â Â Â Â Â Â Âleft_margin = hists__col_len(hists, se->se_width_idx);
> - Â Â Â Â Â Â Â left_margin -= thread__comm_len(self->thread);
> + Â Â Â Â Â Â Â left_margin -= self->thread->comm_len;
> Â Â Â Â}
>
> Â Â Â Âreturn hist_entry_callchain__fprintf(fp, self, session_total,
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index d5d3b22..3ce2a80 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -35,26 +35,18 @@ int thread__set_comm(struct thread *self, const char *comm)
>
> Â Â Â Âif (self->comm)
> Â Â Â Â Â Â Â Âfree(self->comm);
> +
> Â Â Â Âself->comm = strdup(comm);
> +
> Â Â Â Âerr = self->comm == NULL ? -ENOMEM : 0;
> Â Â Â Âif (!err) {
> Â Â Â Â Â Â Â Âself->comm_set = true;
> + Â Â Â Â Â Â Â self->comm_len = strlen(self->comm);
> Â Â Â Â Â Â Â Âmap_groups__flush(&self->mg);
> Â Â Â Â}
> Â Â Â Âreturn err;
> Â}
>
> -int thread__comm_len(struct thread *self)
> -{
> - Â Â Â if (!self->comm_len) {
> - Â Â Â Â Â Â Â if (!self->comm)
> - Â Â Â Â Â Â Â Â Â Â Â return 0;
> - Â Â Â Â Â Â Â self->comm_len = strlen(self->comm);
> - Â Â Â }
> -
> - Â Â Â return self->comm_len;
> -}
> -
> Âstatic size_t thread__fprintf(struct thread *self, FILE *fp)
> Â{
> Â Â Â Âreturn fprintf(fp, "Thread %d %s\n", self->pid, self->comm) +
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index e5f2401..701ea09 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -23,7 +23,6 @@ struct perf_session;
> Âvoid thread__delete(struct thread *self);
>
> Âint thread__set_comm(struct thread *self, const char *comm);
> -int thread__comm_len(struct thread *self);
> Âstruct thread *perf_session__findnew(struct perf_session *self, pid_t pid);
> Âvoid thread__insert_map(struct thread *self, struct map *map);
> Âint thread__fork(struct thread *self, struct thread *parent);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/