Hi,
On Dec 27 2016 19:08, mengdong.lin@xxxxxxxxxxxxxxx wrote:
From: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
To display warnings for us to fix them, add '-Wall' to AM_CPPFLAGS.
Signed-off-by: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
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.html
[1]:
https://www.gnu.org/software/make/manual/html_node/Catalogue-of-Rules.html#Catalogue-of-Rules
[2]:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=test/Makefile.am;h=5f35159af2d64c12e5d394cff884765808d38e5b;hb=HEAD#l29
Regards
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
