[LTP] [PATCH v5 1/3] lib: Add support option for .needs_cmds

[Wei Gao]

On Mon, Dec 15, 2025 at 10:36:39AM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> ...
> > > Also, strictly speaking tst_cmd_present() is only defined, not used. The commit
> > > is about changing .needs_cmds from char array (string) to struct tst_cmd.
> > > Therefore it could be in a separate commit. We don't have to be too strict, but
> > > given how many tests needed to be adapted adding tst_cmd_present() is somehow
> > > buried with other changes.
> 
> > Thanks all for detail feedback. Let's me give some explaination why skip
> > tst_brk in above function:
> > tst_cmd_present will be used in latest ioctl_loop01.c and it should not
> > use tst_brk otherwise we will brk out of test in setup() too early.
> > The new support needs_cmds.optional in current patch is used for support 
> > tst_cmd_present scenario in ioctl_loop01.c.
> > Correct me if any mistake.
> 
> Sure, I noticed tst_cmd_present() usage in a later commit :).
> 
> My point was (while this patchset is also about ideal feature change split into
> commits) that if you touch many files with struct tst_cmd change, adding
> unrelated change (tst_cmd_present()) will hide this change. I would personally
> move adding tst_cmd_present() to a separate commit. It's not that important,
> just to make review easier.
Thanks, i get your point now.
@Petr @Cyril
If we have are agree with following changes then i can sent next patch:
1) Just move the declaration and definition of tst_cmd_present to a separate commit without any functional content changes.
2) Modify function comments such as add ref:`...`
> 
> The rules (which I've tried to explain on this patchset) are
> * each commit must compile (even temporarily break build is not acceptable)
> * split changes into small logical parts
> 
> People talk about atomic changes: "As small as possible, but complete" so that
> they don't break CI.
> https://dev.to/samuelfaure/how-atomic-git-commits-dramatically-increased-my-productivity-and-will-increase-yours-too-4a84
Nice link:)
> 
> Kind regards,
> Petr
> 
> > ioctl_loop01.c code:
> > static void setup(void)
> > {
> >         parted_sup = tst_cmd_present("parted");  <=====
> 
> > .....
> >         if (parted_sup)
> >                 SAFE_CMD(cmd_parted, NULL, NULL);  <===
> 
> >         sprintf(partscan_path, "/sys/block/loop%d/loop/partscan", dev_num);
> > }
> 
> > > Kind regards,
> > > Petr