[m-rev.] for review: shell script fixes

Julien Fischer jfischer at opturion.com
Fri May 29 21:06:06 AEST 2020


Hi Zoltan,

On Fri, 29 May 2020, Zoltan Somogyi wrote:

> I just learned about a linter for shell scripts called shellcheck,
> so I tried it out. It pointed out the issues fixed by this diff,
> as well as other things that aren't problems.
>
> For review by anyone. The main issue is whether the unused
> variables that this diff deletes may nevertheless be needed.
>
> Another issue this brought up for me is: should we prefer
> modern bash-isms such as $(...) over the old Bourne sh
> equivalents such as `...`? Any opinions?

You would need to ensure that any bash-isms you propose to introduce are
likely to work with other Bourne compatible shells in the wild (e.g.
dash etc).

> Fix bugs and nits pointed out by shellcheck.
> 
> scripts/mgnuc.in:
>     Delete the unused variables AS, AS_OPTS, HLD_OPTS and ARG_OPTS.
>     The first two are hstorical relics, HLD_OPTS lost its raison d'etre
>     when we deleted the hl grades; I don't know what we used ARG_OPTS for.
>
>     Make the unused variable CFLAGS_FOR_ANSI used.

We can probably just get rid of it.  The value set by the configure
script is always empty.  (Passing -ansi to GCC puts it into C89 mode,
which isn't particularly useful anymore anyway and I don't think it's
every been useful with any of the other C compilers.)

>     Fix a spelling inconsistency: DEBUG_OPT vs DEBUG_OPTS.
> 
> scripts/ml.in:
>     Delete the unused variables NONSHARED_LIB_DIR and DL_LIBRARY.
> 
> scripts/parse_grade_options.sh-subr:
>     Fix typos that prevented an almost-never-used option from working.
> 
> scripts/parse_ml_options.sh-subr.in:
>     Fix a quoting error.
> 
> tools/bootcheck:
>     Comment out the definition of an unused variable. (Its parallel
>     exists and is used in c2init.in, which is why it is not deleted.)
>
>     Fix typos in spelling SSDB_LIB_NAME.
>
>     Quote a variable value that may contain spaces.
>
>     Replace "cat file | cmd" with "cmd < file".

That looks fine.

Julien.


More information about the reviews mailing list