diff mbox series

[v5,bpf-next,11/15] libbpf: don't use cxx to test_libpf target

Message ID 20191011002808.28206-12-ivan.khoronzhuk@linaro.org
State Accepted
Commit 5c26f9a783581058c5de93627ae64139c4b32ceb
Headers show
Series None | expand

Commit Message

Ivan Khoronzhuk Oct. 11, 2019, 12:28 a.m. UTC
No need to use C++ for test_libbpf target when libbpf is on C and it
can be tested with C, after this change the CXXFLAGS in makefiles can
be avoided, at least in bpf samples, when sysroot is used, passing
same C/LDFLAGS as for lib.

Add "return 0" in test_libbpf to avoid warn, but also remove spaces at
start of the lines to keep same style and avoid warns while apply.

Acked-by: Andrii Nakryiko <andriin@fb.com>

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>

---
 tools/lib/bpf/Makefile                         | 18 +++++-------------
 .../lib/bpf/{test_libbpf.cpp => test_libbpf.c} | 14 ++++++++------
 2 files changed, 13 insertions(+), 19 deletions(-)
 rename tools/lib/bpf/{test_libbpf.cpp => test_libbpf.c} (61%)

-- 
2.17.1

Comments

Stanislav Fomichev Nov. 23, 2019, 12:01 a.m. UTC | #1
On 11/23, Ivan Khoronzhuk wrote:
> On Fri, Nov 22, 2019 at 08:32:11AM -0800, Stanislav Fomichev wrote:

> > On 11/21, Andrii Nakryiko wrote:

> > > On Thu, Nov 21, 2019 at 1:42 PM Stanislav Fomichev <sdf@fomichev.me> wrote:

> > > >

> > > > On 10/11, Ivan Khoronzhuk wrote:

> > > > > No need to use C++ for test_libbpf target when libbpf is on C and it

> > > > > can be tested with C, after this change the CXXFLAGS in makefiles can

> > > > > be avoided, at least in bpf samples, when sysroot is used, passing

> > > > > same C/LDFLAGS as for lib.

> > > > > Add "return 0" in test_libbpf to avoid warn, but also remove spaces at

> > > > > start of the lines to keep same style and avoid warns while apply.

> > > > Hey, just spotted this patch, not sure how it slipped through.

> > > > The c++ test was there to make sure libbpf can be included and

> > > > linked against c++ code (i.e. libbpf headers don't have some c++

> > > > keywords/etc).

> > > >

> > > > Any particular reason you were not happy with it? Can we revert it

> > > > back to c++ and fix your use-case instead? Alternatively, we can just

> > > > remove this test if we don't really care about c++.

> > > >

> > > 

> > > No one seemed to know why we have C++ pieces in pure C library and its

> > > Makefile, so we decide to "fix" this. :)

> > It's surprising, the commit 8c4905b995c6 clearly states the reason

> > for adding it. Looks like it deserved a real comment in the Makefile :-)

> 

> I dislike changing things like this, but I was asked while review and

> it seemed logical enough. The comment could prevent us from doing this.

No worries, I'll add it back with a comment :-)

> > > But I do understand your concern. Would it be possible to instead do

> > > this as a proper selftests test? Do you mind taking a look at that?

> > Ack, will move this test_libbpf.c into selftests and convert back to

> > c++.

> 

> -- 

> Regards,

> Ivan Khoronzhuk
diff mbox series

Patch

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 1270955e4845..46280b5ad48d 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -52,7 +52,7 @@  ifndef VERBOSE
 endif
 
 FEATURE_USER = .libbpf
-FEATURE_TESTS = libelf libelf-mmap bpf reallocarray cxx
+FEATURE_TESTS = libelf libelf-mmap bpf reallocarray
 FEATURE_DISPLAY = libelf bpf
 
 INCLUDES = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(ARCH)/include/uapi -I$(srctree)/tools/include/uapi
@@ -142,15 +142,7 @@  GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \
 VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \
 			      grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
 
-CMD_TARGETS = $(LIB_TARGET) $(PC_FILE)
-
-CXX_TEST_TARGET = $(OUTPUT)test_libbpf
-
-ifeq ($(feature-cxx), 1)
-	CMD_TARGETS += $(CXX_TEST_TARGET)
-endif
-
-TARGETS = $(CMD_TARGETS)
+CMD_TARGETS = $(LIB_TARGET) $(PC_FILE) $(OUTPUT)test_libbpf
 
 all: fixdep
 	$(Q)$(MAKE) all_cmd
@@ -190,8 +182,8 @@  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
 $(OUTPUT)libbpf.a: $(BPF_IN)
 	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
 
-$(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
-	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
+$(OUTPUT)test_libbpf: test_libbpf.c $(OUTPUT)libbpf.a
+	$(QUIET_LINK)$(CC) $(INCLUDES) $^ -lelf -o $@
 
 $(OUTPUT)libbpf.pc:
 	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
@@ -266,7 +258,7 @@  config-clean:
 	$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ clean >/dev/null
 
 clean:
-	$(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \
+	$(call QUIET_CLEAN, libbpf) $(RM) $(CMD_TARGETS) \
 		*.o *~ *.a *.so *.so.$(LIBBPF_MAJOR_VERSION) .*.d .*.cmd \
 		*.pc LIBBPF-CFLAGS bpf_helper_defs.h
 	$(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
diff --git a/tools/lib/bpf/test_libbpf.cpp b/tools/lib/bpf/test_libbpf.c
similarity index 61%
rename from tools/lib/bpf/test_libbpf.cpp
rename to tools/lib/bpf/test_libbpf.c
index fc134873bb6d..f0eb2727b766 100644
--- a/tools/lib/bpf/test_libbpf.cpp
+++ b/tools/lib/bpf/test_libbpf.c
@@ -7,12 +7,14 @@ 
 
 int main(int argc, char *argv[])
 {
-    /* libbpf.h */
-    libbpf_set_print(NULL);
+	/* libbpf.h */
+	libbpf_set_print(NULL);
 
-    /* bpf.h */
-    bpf_prog_get_fd_by_id(0);
+	/* bpf.h */
+	bpf_prog_get_fd_by_id(0);
 
-    /* btf.h */
-    btf__new(NULL, 0);
+	/* btf.h */
+	btf__new(NULL, 0);
+
+	return 0;
 }