Re: [LTP] [PATCH 2/6] tst_env.sh: Backport common functions from tst_test.sh

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> Hi!
> > > SAFE instead so that the name is closer to the SAFE_XXX macros in C.

> > Makes sense. I was even thinking on SAFE_CMD, but we use tst_rod.c, not
> > tst_cmd.c. OTOH it was obvious that ROD uses tst_rod.c, should it be also
> > renamed? What ROD means anyway? run or dye?

> Run or die (we are not coloring anything).

> And yes it would make sense to rename tst_rod.c to tst_safe_cmd.c or
> similar.

+1, I'll do in next version.

> > > > +_tst_expect_pass()
> > > > +{
> > > > +	local fnc="$1"
> > > > +	shift
> > > > +
> > > > +	tst_rod "$@"

> > > If I remmeber correctly the whole reason why we introduced tst_rod.c was
> > > that passing the $@ like this causes the $@ to be evaluated twice and
> > > produces unexpected results.

> > FYI code is copy pasted from tst_test.sh. What do you want me to change?

> I've spend a couple of minutes figuring out why we originaly implemented
> ROD_BASE() in C.

> The problem is that single quotes gets removed as soon as we pass the
> parameter.

> E.g.: ROD awk '{ print $1 }'

> Once we pass that to a shell function the '' disappear, they managed to
> keep the { print $1 } to be a single parameter in the list but they are
> not there anymore.

Yeah, I've found this shell limitation in the past as well.

> And since we were manipulating the parameter list we got, we failed to
> reconstruct it correctly in shell since in the next step the { print $1 }
> breaks into four separate strings.

> As long as we pass the $@ as a whole (even after shift), it does work
> since the boundaries of parameters in $@ stay exactly as they were.

> TL;DR; The code you copy&pasted is correct, I only misremembered what
> was the problem back then.

Good, I was worried it was something else needed to be fixed. Thanks for double
checking!

> > > I'm not sure that adding the PASS_BRK and FAIL_BRK is a good idea. I
> > > would stick to simple EXPECT_PASS and EXPECT_FAIL. And maybe we can
> > > export TST_PASS variable as we do in C to match the API. I think that
> > > the closer the C and shell API are the better.

> > C API usually returns on if (!TST_PASS). I don't like it either, but it cannot
> > be shortened much as we don't use functions but macros.

> > But changing the code in the shell API does not look to me an improvement:

> > -EXPECT_PASS_BRK ping$TST_IPV6 -c1 -I $lhost $rhost \>/dev/null
> > +EXPECT_PASS_BRK ping$TST_IPV6 -c1 -I $lhost $rhost \>/dev/null
> > +[ "$TST_PASS" = 1 ] || tst_brk "Quit due the above error"

> Let's keep it then.

+1. We can revisit it later. Anyway, I'll add only what's currently needed,
maybe these network tests will be changed or rewritten into C.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp