Re: [LTP] [PATCH v5 1/3] Refactor regen.sh script to generate syscalls

[Petr Vorel <pvorel@suse.cz>]

> Hi Petr,

> On 10/30/24 15:49, Petr Vorel wrote:
> > Hi Andrea,

> > > +++ b/include/lapi/syscalls/generate_syscalls.sh
> > > @@ -0,0 +1,115 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +#
> > > +# Generate the syscalls.h file, merging all architectures syscalls input file
> > > +# which are in the current folder and defined inside supported-arch.txt file.
> > > +
> > > +SYSCALLS_FILE="$1"
> > > +
> > > +if [ -z "${SYSCALLS_FILE}" ]; then
> > nit: I would still replace ${...} with $... (e.g. ${SYSCALLS_FILE} =>
> > $SYSCALLS_FILE) as I noted in v4 - readability. IMHO there is no need to use
> > ${...} in whole script.

> > The same applies to include/lapi/syscalls/generate_arch.sh in the second commit.

> This is a bit debatable. The reason why using ${VAR} over $VAR is the
> readability in the first place (i.e. when you have `${PATH}/bin` instead
> `$PATH/bin` or when you use characters which could mix with PATH var name
> and it might create debug issues), but also the fact it provides
> ${VAR:-default} syntax which can always added afterwards.

> I usually prefer this method for these reasons.

Sure, it's matter of preference. At least LTP shell library tst_test.sh
is happy with $VAR instead of ${VAR}. But as generate_arch.sh origin is in the
separate project, and this is a separate shell script (not part of shell API)
let's ignore my suggestion.

Kind regards,
Petr

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