Hi,
On Dec 27 2016 19:08, mengdong.lin@linux.intel.com wrote:
From: Mengdong Lin mengdong.lin@linux.intel.com
To display warnings for us to fix them, add '-Wall' to AM_CPPFLAGS.
Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com
diff --git a/src/ucm/Makefile.am b/src/ucm/Makefile.am index 9d66b24..79e57d2 100644 --- a/src/ucm/Makefile.am +++ b/src/ucm/Makefile.am @@ -7,4 +7,4 @@ noinst_HEADERS = ucm_local.h all: libucm.la
-AM_CPPFLAGS=-I$(top_srcdir)/include +AM_CPPFLAGS=-I$(top_srcdir)/include -Wall
As long as I know, the '-Wall' option is for C/C++ compiler. Therefore, it's not necessarily correct to add the option to 'AM_CPPFLAGS' as options for preprocessor. In a manual of GNU Automake, there're some samples for this option; '-I' and '-D'[0]. Usage of 'AM_CFLAGS' might be better in this case.
...But this indication is a bit nitpicking and I'm not so strongly oppose this patch. I'll manage to describe the reason.
In a manual of GNU compiler collection, there're samples of implicit rules for C/C++ compilers: * $(CC) $(CPPFLAGS) $(CFLAGS) -c * $(CXX) $(CPPFLAGS) $(CXXFLAGS) -c
If the place of 'CPPFLAGS' is replaced with options in AM_CPPFLAGS, your patch has an effect for both of C code and C++ code. This is somehow expected for some developers, while it's also be obtrusive because of sub-effect to C++ code in our case.
Currently, alsa-lib includes no C++ codes, while this patch could bring the obtrusiveness for future when some C++ codes were added (i.e. by code refactoring). But the result were preferable by itself, I believe.
Well, if Makefile.am includes 'mumble_CFLAGS' lines, options in 'AM_CFLAGS' is ignored to generate the 'mumble'. For example, a command to generate 'user_ctl_element_set' object in 'test' directory is this case (I wrote it[2]).
Currently I cannot judge which way is better... I'm sorry but could I request your opinion for this patch with the view of these specifications?
[0]: https://www.gnu.org/software/automake/manual/html_node/Program-Variables.htm... [1]: https://www.gnu.org/software/make/manual/html_node/Catalogue-of-Rules.html#C... [2]: http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=test/Makefile.am;h=5f35...
Regards
Takashi Sakamoto