From: Pratyush Yadav <pratyush@kernel.org>
To: Vipin Sharma <vipinsh@google.com>
Cc: pasha.tatashin@soleen.com, rppt@kernel.org,
pratyush@kernel.org, shuah@kernel.org, dmatlack@google.com,
skhawaja@google.com, tarunsahu@google.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 1/2] selftests/liveupdate: Move luo_test_utils.* into a reusable library
Date: Mon, 08 Jun 2026 16:31:57 +0200 [thread overview]
Message-ID: <2vxzo6hlt86a.fsf@kernel.org> (raw)
In-Reply-To: <20260511201155.1488670-2-vipinsh@google.com> (Vipin Sharma's message of "Mon, 11 May 2026 13:11:54 -0700")
On Mon, May 11 2026, Vipin Sharma wrote:
> Move luo_test_utils.[ch] into a lib/ directory and pull the rules to
> build them out into a separate make script. This will enable these
> utilities to be also built by and used within other selftests (such as
> VFIO).
>
> No functional change intended.
>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> Co-developed-by: David Matlack <dmatlack@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
> tools/testing/selftests/liveupdate/.gitignore | 1 +
> tools/testing/selftests/liveupdate/Makefile | 14 ++++---------
> .../include/libliveupdate.h} | 8 ++++----
Nit: perhaps libluo is a bit less wordy?
No strong opinions on it though, so I am fine with anything.
[...]
> diff --git a/tools/testing/selftests/liveupdate/lib/libliveupdate.mk b/tools/testing/selftests/liveupdate/lib/libliveupdate.mk
> new file mode 100644
> index 000000000000..fffd95b085b6
> --- /dev/null
> +++ b/tools/testing/selftests/liveupdate/lib/libliveupdate.mk
> @@ -0,0 +1,20 @@
> +include $(top_srcdir)/scripts/subarch.include
> +ARCH ?= $(SUBARCH)
> +
> +LIBLIVEUPDATE_SRCDIR := $(selfdir)/liveupdate/lib
> +
> +LIBLIVEUPDATE_C := liveupdate.c
> +
> +LIBLIVEUPDATE_OUTPUT := $(OUTPUT)/libliveupdate
> +
> +LIBLIVEUPDATE_O := $(patsubst %.c, $(LIBLIVEUPDATE_OUTPUT)/%.o, $(LIBLIVEUPDATE_C))
> +
> +LIBLIVEUPDATE_O_DIRS := $(shell dirname $(LIBLIVEUPDATE_O) | uniq)
> +$(shell mkdir -p $(LIBLIVEUPDATE_O_DIRS))
Sashiko complains about this:
https://sashiko.dev/#/patchset/20260511201155.1488670-1-vipinsh%40google.com
Does this execute the directory creation unconditionally during Make parsing?
Because it uses the shell function at the top level, this directory creation
will run on every Make invocation, including utility targets like make clean
or make help.
Could the shell dirname command also be fragile if LIBLIVEUPDATE_O ever
expands to multiple files? Strictly POSIX-compliant systems only accept a
single argument for dirname.
Would it be better to use GNU Make's built-in $(dir ...) function and move
the directory creation into the compilation recipe below (e.g.,
@mkdir -p $(dir $@)), or use an order-only prerequisite?
The first one seems to make sense. The second one not so much. The third
I have no idea. I don't know Makefiles well enough.
Perhaps worth a look?
With these comments addressed, feel free to add
Acked-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> +
> +CFLAGS += -I$(LIBLIVEUPDATE_SRCDIR)/include
> +
> +$(LIBLIVEUPDATE_O): $(LIBLIVEUPDATE_OUTPUT)/%.o : $(LIBLIVEUPDATE_SRCDIR)/%.c
> + $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> +
> +EXTRA_CLEAN += $(LIBLIVEUPDATE_OUTPUT)
[...]
--
Regards,
Pratyush Yadav
next prev parent reply other threads:[~2026-06-08 14:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-11 20:11 [PATCH 0/2] Make liveupdate selftests util as a library Vipin Sharma
2026-05-11 20:11 ` [PATCH 1/2] selftests/liveupdate: Move luo_test_utils.* into a reusable library Vipin Sharma
2026-06-08 14:31 ` Pratyush Yadav [this message]
2026-05-11 20:11 ` [PATCH 2/2] selftests/liveupdate: Add helpers to preserve/retrieve FDs Vipin Sharma
2026-05-18 11:29 ` tarunsahu
2026-05-18 17:14 ` Vipin Sharma
2026-06-08 14:32 ` Pratyush Yadav
2026-06-08 18:44 ` tarunsahu
2026-06-08 14:38 ` Pratyush Yadav
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2vxzo6hlt86a.fsf@kernel.org \
--to=pratyush@kernel.org \
--cc=dmatlack@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=pasha.tatashin@soleen.com \
--cc=rppt@kernel.org \
--cc=shuah@kernel.org \
--cc=skhawaja@google.com \
--cc=tarunsahu@google.com \
--cc=vipinsh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox