Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eval evil? #59

Open
coderofsalvation opened this issue Sep 3, 2018 · 7 comments
Open

eval evil? #59

coderofsalvation opened this issue Sep 3, 2018 · 7 comments

Comments

@coderofsalvation
Copy link
Owner

coderofsalvation commented Sep 3, 2018

So from what i've understood is that the compiler is using eval, to stay flexible concerning bash vs sh output.
So powscript generally produces this type of output:

set?() {
  {
    local s;
    [ $# -ge 1 ] && s="${1}"
  }
  eval "{ return \"\$(( \${$s+-1}+1 ))\"; }"
}

@fcard: Might there be a way to get rid of the eval you think?

Could we get away with this kind of output in most cases? :

set?() {
  {
    local s;
    [ $# -ge 1 ] && s="${1}"
  }
  return "$(( ${$s+-1}+1 ))"
}

That way powscript can also be attractive to people who dismiss the usage of eval (because of security reasons).
Looking at all my powscript-output, I haven't really seen a case where wrapping shellscript in an eval-call was necessary.
WDYT?

@fcard
Copy link
Collaborator

fcard commented Sep 3, 2018

Unfortunately we can't do${$s} (gives a "bad substitution" error), which is exactly why we need eval when dealing with names and arrays, otherwise I'd agree with you that we should avoid eval.

Since we have expand for these cases, we can make it more secure, and recommend it over directly using eval whenever possible.

@coderofsalvation
Copy link
Owner Author

coderofsalvation commented Sep 4, 2018

i see.
Maybe a silly question, but why would we want to do ${$s} if we can do ${s}?
And if the powscript transpiler can output any text, shouldn't it just be able to output something like this:

set?(){
  {
    local s;
    [ $# -ge 1 ] && s="${1}"
  }
  return $?           # will return resultcode of last executed command 
}

or ideally:

set?(){
  [ $# -ge 1 ] && return 0 || return 1
}

I'm hoping that we've just discovered a lowhanging fruit here :)

@fcard
Copy link
Collaborator

fcard commented Sep 4, 2018

Your version would print "true" in the below code:

unset s

if set? s
  echo "true"

We should probably add tests to the std lib soon, so we can experiment better :P

The reason we need an equivalent to ${$s} is because we're dealing with names (or references) instead of strings.

@coderofsalvation
Copy link
Owner Author

coderofsalvation commented Sep 4, 2018

Afaik in shellscript reference-notation (without $) is only used in (undoing) declarations & assignments, not in if-statements etc.
As confusing as that already is, maybe we should stick to that? (with function arguments as exception?)
Or are there other cases which require us to stick with eval?

unset s

if set? $s
   echo "true" # will not print

Let me know what u think.

@fcard
Copy link
Collaborator

fcard commented Sep 4, 2018

But that will print false below.

s=""

if set? $s
  echo true
else
  echo false

There is no meaning in set? without references.
There are many uses in references, they are normally just too annoying in shell scripting to use :P

@fcard
Copy link
Collaborator

fcard commented Sep 4, 2018

Actually, that will print true, I forgot about auto quoting. However, that means that

unset s

if set? $s
   echo "true"

Will print true with your version.

@coderofsalvation
Copy link
Owner Author

coderofsalvation commented Sep 4, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants