Re: [RFC PATCH 4/4] kbuild: evaluate cc-option and friends only when building kernel

From: Doug Anderson
Date: Mon Oct 09 2017 - 18:04:10 EST


Hi,

On Tue, Oct 3, 2017 at 8:56 PM, Masahiro Yamada
<yamada.masahiro@xxxxxxxxxxxxx> wrote:
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 9ffd3dd..222d0a2 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -96,6 +96,13 @@ try-run = $(shell set -e; \
> fi; \
> rm -f "$$TMP" "$$TMPO")
>
> +# hostcc-option
> +# Usage: cflags-y += $(call hostcc-option,-march=winchip-c6,-march=i586)
> +hostcc-option = $(call __cc-option, $(HOSTCC),\
> + $(HOSTCFLAGS) $(HOST_EXTRACFLAGS),$(1),$(2))

I believe you've got a bug here. You're calling "__cc-option" which
isn't defined if "need-compiler" is not 1, right?

> +
> +ifeq ($(need-compiler),1)

The way this patch works is a bit non-obvious I think. I wonder if
anyone else will be confused like I am...

Basically if "need-compiler" is not 1 then things like "cc-option"
won't be defined at all. ...but we'll still _call_ them in other
Makefiles. This call of an undefined variable will just evaluate to
an empty string. Thus, for instance:

CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,)

...will just set CFLAGS_KCOV to the empty string if "need-compiler"
isn't 1, right?


I guess that's fine, but maybe at least document it somewhere? IMHO
it would be even better if somehow you still defined each of these to
something bogus in an else clause, like:

as-option = --err_noncompile_target
as-instr = --err_noncompile_target
cc-option = --err_noncompile_target
...
...

The idea being that if someone accidentally invoked the C compiler (or
if there was some other conditional code in the Makefile based on the
result of one of these functions) it would be obvious what was going
on.


-Doug