On 07/09/2022 00:31, Song Liu wrote: > On Tue, Sep 6, 2022 at 6:44 AM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote: >> > [...] > >> >> +# If one of the above feature combinations is set, we support libbfd >> ifneq ($(filter -lbfd,$(LIBS)),) >> -CFLAGS += -DHAVE_LIBBFD_SUPPORT >> -SRCS += $(BFD_SRCS) >> + CFLAGS += -DHAVE_LIBBFD_SUPPORT >> + >> + # Libbfd interface changed over time, figure out what we need >> + ifeq ($(feature-disassembler-four-args), 1) >> + CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE >> + endif >> + ifeq ($(feature-disassembler-init-styled), 1) >> + CFLAGS += -DDISASM_INIT_STYLED >> + endif >> +endif > > >> +ifeq ($(filter -DHAVE_LIBBFD_SUPPORT,$(CFLAGS)),) >> + # No support for JIT disassembly >> + SRCS := $(filter-out jit_disasm.c,$(SRCS)) >> endif > > This part could just be an else clause for the ifneq above. > Well, I guess the difference is minimal. True for this patch, but please see patch 6 with the LLVM support: the ifneq above gets embedded in an outer if/else block (we only run it if LLVM is not found), whereas removing jit_disasm.c from the sources occurs when none of the two libs is available. Ideally we'd have "if LLVM ... else if libbfd ... else remove jit_disasm.c", but the check on libbfd involved checking multiple features so I didn't find a simple way to write that in Makefile syntax and thought it more readable to have a separate block for jit_disasm.c. > > Acked-by: Song Liu <song@xxxxxxxxxx> Thanks for the review!
