On 2018-03-19 03:38 +0000, Wookey wrote:
On 2018-03-16 16:54 +0000, Mike Leach wrote: I'll do the 'mechanism to pass in flags' thing next.
OK. This is the simplest patch that allow passing-in of external flags (by defining the flags on the make command line):
Index: libopencsd-0.8.1/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/makefile +++ libopencsd-0.8.1/decoder/build/linux/makefile @@ -76,9 +76,12 @@ INSTALL_LIB_DIR=$(PREFIX)/lib/$(DEB_HOST export INSTALL_INCLUDE_DIR=$(PREFIX)/include
# compile flags -MASTER_CC_FLAGS := -c -Wall -DLINUX -MASTER_CXX_FLAGS := -c -Wall -DLINUX -std=c++11 -MASTER_LINKER_FLAGS := -Wl,-z,defs +CFLAGS += -c -Wall -DLINUX +CXXFLAGS += -c -Wall -DLINUX -std=c++11 +LDFLAGS += -Wl,-z,defs +MASTER_CC_FLAGS := $(CFLAGS) +MASTER_CXX_FLAGS := $(CXXFLAGS) +MASTER_LINKER_FLAGS := $(LDFLAGS) MASTER_LIB_FLAGS := rcs
# debug variant
The current flags used in various places are: CC_FLAGS (CFLAGS) := -c -Wall -DLINUX CXX_FLAGS (CXXFLAGS):= -c -Wall -DLINUX -std=c++11 LINKER_FLAGS (LDFLAGS) := -Wl,-z,defs LIB_FLAGS (ARFLAGS) := rcs
rctdl_c_api_lib: CXXFLAGS += -fpic -Wno-switch LDFLAGS += -shared
ref_trace_decode_lib: CXXFLAGS += -fpic -Wno-switch LDFLAGS += -shared
echo_test_dcd_lib: CFLAGS += -fpic -Wno-switch ARFLAGS
snapshot_parser_lib: CFLAGS += -fpic -Wno-switch -Wno-deprecated-declarations -Wno-unused-variable -Wno-reorder ARFLAGS
c_api_pkt_print_test: CFLAGS += -Wno-switch LDFLAGS += -Wl,--start-group $(LIBS) -Wl,--end-group
trc_pkt_lister: CXXFLAGS += -Wno-switch LDFLAGS
So that shows that -Wno-switch should be in the defaults -fpic is not used in two places, but in fact this is now the compiler default, at least in debian, and enabling it everywhere seems to be fine. Was there some reason why it was turned off for c_api_pkt_print_test and trc_pkt_lister?
LDFLAGS is used without -shared in two places, and those seem to make sense, so putting -shared on the 'link .so' lines, the special -Wl,--start-group in the one place it's needed and just using LDFLAGS default everywhere works fine.
So there is no reason not to use standard flag variable names thoughout and skip the MASTER->local copying. I've done this, using these defaults: CFLAGS := $(CPPFLAGS) -c -Wall -DLINUX -Wno-switch -fpic CXXFLAGS := $(CPPFLAGS) -c -Wall -DLINUX -Wno-switch -fpic -std=c++11 LDFLAGS := -Wl,-z,defs ARFLAGS := rcs
The attached patch makes these changes, allowing extra options to be passed in from outside.
One other thing. Should we enable --as-needed in the link? I've done this for the debian package but if it's a good idea it should go upstream.
Wookey