Re: [PATCH] perf: fix wrong DEBUG configuration

From: Arnaldo Carvalho de Melo
Date: Mon May 25 2015 - 09:52:49 EST


Em Mon, May 25, 2015 at 01:12:44PM +0200, Martin LiÅka escreveu:
> On 05/25/2015 01:09 PM, Ingo Molnar wrote:
> >
> >* Martin LiÅka <mliska@xxxxxxx> wrote:
> >
> >>>>Right optimize debugging experience is given by passing -Og to
> >>>>compiler. Assign default value for pointers that are identified by
> >>>>compiler as non-initialized.
> >>>
> >>>s/Right optimize debugging experience is given/
> >>> Correct debugging experience is given/
> >
> >>Right debugging experience is given by passing -Og to compiler.
> >
> >So I fixed the spelling here once already :-/ If you want to use
> >'right' in this context then use it like this:
> >
> > The right debugging experience is given by ...
> >
> >Or you can use what I suggested first:
> >
> > Correct debugging experience is given by ...
> >
> >Thanks,
> >
> > Ingo
> >
>
> Sorry Ingo for that, I overlooked that correction :)

Are you ok with this Ingo?

I can apply, but there seems to be two patches folded here, one that
sets the possibly unitiliazed variables to NULL, and could be the first
in the series, and the other, that deals with multiple versions of gcc
and how should we ask something from them.

- Arnaldo

> Attached version should reflect that.
>
> Thanks,
> Martin

> Currently, GCC optimizes -O6 same as -O3 level, thus change the value
> to -O3.
> Correct debugging experience is given by passing -Og to compiler.
> Assign default value for pointers that are identified by the compiler as
> non-initialized.
>
> Signed-off-by: Martin Liska <mliska@xxxxxxx>
> Acked-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
> tools/perf/arch/common.c | 2 +-
> tools/perf/config/Makefile | 4 +++-
> tools/perf/config/utilities.mak | 19 +++++++++++++++++++
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/trace-event-parse.c | 2 +-
> 5 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
> index 49776f1..b7bb42c 100644
> --- a/tools/perf/arch/common.c
> +++ b/tools/perf/arch/common.c
> @@ -61,7 +61,7 @@ const char *const mips_triplets[] = {
> static bool lookup_path(char *name)
> {
> bool found = false;
> - char *path, *tmp;
> + char *path, *tmp = NULL;
> char buf[PATH_MAX];
> char *env = getenv("PATH");
>
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index e3b3724..47e4ae1 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -129,7 +129,9 @@ ifndef DEBUG
> endif
>
> ifeq ($(DEBUG),0)
> - CFLAGS += -O6
> + CFLAGS += -O3
> +else
> + CFLAGS += $(call cc-option,-Og,-O0)
> endif
>
> ifdef PARSER_DEBUG
> diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak
> index c16ce83..0ebef09 100644
> --- a/tools/perf/config/utilities.mak
> +++ b/tools/perf/config/utilities.mak
> @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
> endef
> _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
> _gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
> +
> +# try-run
> +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
> +# Exit code chooses option. "$$TMP" is can be used as temporary file and
> +# is automatically cleaned up.
> +try-run = $(shell set -e; \
> + TMP="$(TMPOUT).$$$$.tmp"; \
> + TMPO="$(TMPOUT).$$$$.o"; \
> + if ($(1)) >/dev/null 2>&1; \
> + then echo "$(2)"; \
> + else echo "$(3)"; \
> + fi; \
> + rm -f "$$TMP" "$$TMPO")
> +
> +# cc-option
> +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
> +
> +cc-option = $(call try-run,\
> + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 82a31fd..a19fbd4 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -400,7 +400,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> const char *name)
> {
> struct rb_node *n;
> - struct symbol_name_rb_node *s;
> + struct symbol_name_rb_node *s = NULL;
>
> if (symbols == NULL)
> return NULL;
> diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
> index 25d6c73..d495741 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent,
> char *line;
> char *next = NULL;
> char *addr_str;
> - char *fmt;
> + char *fmt = NULL;
>
> line = strtok_r(file, "\n", &next);
> while (line) {
> --
> 2.1.4
>

--
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/