Re: [PATCH 1/3] rtla: Add libsubcmd dependency

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Low] The variable `LIBSUBCMD_INCLUDES` is incorrectly used as both a compiler flag and a Make target, causing unnecessary sub-make executions.
- [Low] The manual compilation rules for `string.o` and `str_error_r.o` bypass the standard `fixdep` dependency tracking mechanism.
--

commit 61af24c97cebeb1e051afb96f7fccb2a5c359a72
Author: Tomas Glozar <tglozar@redhat.com>

rtla: Add libsubcmd dependency

This commit prepares RTLA to migrate to libsubcmd by building and linking
the library alongside the RTLA build. It also links respective libraries
from tools/lib and adds tools/include to the include path.

> diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
> index 45690ee14544d..289e44c9664b2 100644
> --- a/tools/tracing/rtla/Makefile
> +++ b/tools/tracing/rtla/Makefile
> @@ -27,6 +27,24 @@ endif
>  RTLA		:= $(OUTPUT)rtla
>  RTLA_IN		:= $(RTLA)-in.o
>  
> +LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
> +ifneq ($(OUTPUT),)
> +  LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> +else
> +  LIBSUBCMD_OUTPUT = $(CURDIR)/libsubcmd
> +endif
> +LIBSUBCMD = $(LIBSUBCMD_OUTPUT)/libsubcmd.a
> +LIBSUBCMD_INCLUDES = -I$(LIBSUBCMD_OUTPUT)/include

[ ... ]

> -$(RTLA_IN): fixdep FORCE src/timerlat.skel.h
> +$(RTLA_IN): fixdep FORCE src/timerlat.skel.h $(LIBSUBCMD_INCLUDES)
>  	make $(build)=rtla
>  
> -clean: doc_clean fixdep-clean
> +$(LIBSUBCMD_OUTPUT):
> +	$(Q)$(MKDIR) -p $@
> +
> +$(LIBSUBCMD_INCLUDES): $(LIBSUBCMD_OUTPUT)
> +	$(Q)$(MAKE) -C $(LIBSUBCMD_DIR) $(LIBSUBCMD_MAKEFLAGS) \
> +		install_headers

Does this unintentionally cause the install_headers sub-make to run on
every build?

Since LIBSUBCMD_INCLUDES expands to the compiler flag string
-I$(LIBSUBCMD_OUTPUT)/include, make interprets this literal string as a
target. Because a file named "-I..." is never actually created by the
install_headers step, make considers the target permanently out-of-date
and executes this recipe unconditionally on every invocation.

> +$(LIBSUBCMD): fixdep $(LIBSUBCMD_OUTPUT)
> +	$(Q)$(MAKE) -C $(LIBSUBCMD_DIR) $(LIBSUBCMD_MAKEFLAGS) \
> +		$@
> +
> +$(LIB_STR_ERROR_R): $(LIB_STR_ERROR_R_SRC)
> +	$(QUIET_CC)$(CC) $(CFLAGS) -c -o $@ $<
> +
> +$(LIB_STRING): $(LIB_STRING_SRC)
> +	$(QUIET_CC)$(CC) $(CFLAGS) -c -o $@ $<

Do these manual compilation rules bypass dependency tracking?

These targets invoke the compiler directly without using the standard
tools/build/Makefile.build infrastructure. By omitting the necessary
compiler flags to generate dependency files and skipping the fixdep
post-processing, updates to any headers included by string.c or
str_error_r.c will not automatically trigger a rebuild of these object
files.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260320150651.51057-1-tglozar@redhat.com?part=1