Re: [PATCH v4 1/4] tools: Add Makefile.include

From: Sam Ravnborg
Date: Tue Apr 10 2012 - 16:27:21 EST


Hi Borislav.

Some random comments as I really did not look at this
part of the patch-set before.

> +#
> +# Include saner warnings here, which can catch bugs:
> +#
> +EXTRA_WARNINGS := -Wformat
> +EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security

Two general comments - and I know this is just something you copied...
Why not use the += operator. The below looks like shell script syntax.
And we use the += operator in other places - at least in the kernel stuff.

AND WHY ALL THESE SCREAMING CAPITAL LETTERS?
I know Makefiles and scripts are full of them - but that does not help
the readability.

For kbuild I generally shifted to use:
- lower-case names for local stuff
- Upper case letters for global stuff - properly prefixed to avoid collisions
EXTRA_WARNINGS likely fall into the category global-stuff,
but then a local variable could still be usefull.

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