Am Mittwoch, 20. September 2017, 12:50:26 CEST schrieb Heiko Bauke:
> Hi,

Hi.

> at the moment I am reviewing the shell scrips in the darktable source,
> mainly by employing the shellcheck tool.  In this way, I found some
> minor (manly stylistic) issues.

The manly stylistic issues. It's all about beard length, isn't it? ;-)

> So far I found only one more serious issue in the statement
> 
> if [ -n  "`echo -e $NEW_VERSION | grep  Format`" ]; then
> 
> in the file tools/create_version_c.sh.  The problem is that POSIX
> compliant implementations of the build in echo shell function do not
> support options.  Thus
> 
> echo -e $NEW_VERSION
> 
> will print out literally »-e« plus the content of the variable
> $NEW_VERSION (with the usual parameter expansion rules applied) in some
> shells (the dash, for example, which is the default on Debian
> derivatives).  This, however, might not be intended here.  Some
> implementations of echo (the bash build in, for example) allow to
> specify the option -e to enable the interpretation of backslash escapes.

Ack, that looks wrong. The interesting thing is that in the end it doesn't 
really matter if there is a stray "-e" but it's bad nonetheless.

> Thus, I wonder do we really need to enable the interpretation of
> backslash escapes here?  If yes echo should be replaced by /bin/echo,
> otherwise »-e« is probably wrong here. Probably the line
> 
> if [ -n  "`echo -e $NEW_VERSION | grep  Format`" ]; then
> 
> should be replaced by
> 
> if echo "$NEW_VERSION" | grep -q Format; then
> 
> but I am not sure.

I pinged Jeremy who introduced those changes back then. Maybe they were a 
typo, maybe there was a reason. Let's wait for his input.

>       Heiko

Tobias

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to