eval() should be avoided when at all possible

eval can have security consequences.  It doesn't look bad here but it
does introduce unnecessary complexity and would make it harder if we
ever want to use static analysis to detect and prohibit eval.  So we
should get rid of it.

Note: this could be even more efficient if we combined the checks into
a single condition instead of looping but that does change the error
messages a bit.  For instance:
-        for arg in ('name', 'linode_id'):
-            if not eval(arg):
+        if not (name and linode_id):
+            module.fail_json(msg='name and linode_id are required for active state')
This commit is contained in:
Toshio Kuratomi 2017-01-04 19:01:43 -08:00
parent ed624876d1
commit 22c9428776

View file

@ -250,8 +250,8 @@ def linodeServers(module, api, state, name, plan, distribution, datacenter, lino
# Any create step triggers a job that need to be waited for.
if not servers:
for arg in ('name', 'plan', 'distribution', 'datacenter'):
if not eval(arg):
for arg in (name, plan, distribution, datacenter):
if not arg:
module.fail_json(msg='%s is required for active state' % arg)
# Create linode entity
new_server = True
@ -267,8 +267,8 @@ def linodeServers(module, api, state, name, plan, distribution, datacenter, lino
module.fail_json(msg = '%s' % e.value[0]['ERRORMESSAGE'])
if not disks:
for arg in ('name', 'linode_id', 'distribution'):
if not eval(arg):
for arg in (name, linode_id, distribution):
if not arg:
module.fail_json(msg='%s is required for active state' % arg)
# Create disks (1 from distrib, 1 for SWAP)
new_server = True
@ -300,8 +300,8 @@ def linodeServers(module, api, state, name, plan, distribution, datacenter, lino
module.fail_json(msg = '%s' % e.value[0]['ERRORMESSAGE'])
if not configs:
for arg in ('name', 'linode_id', 'distribution'):
if not eval(arg):
for arg in (name, linode_id, distribution):
if not arg:
module.fail_json(msg='%s is required for active state' % arg)
# Check architecture
@ -387,8 +387,8 @@ def linodeServers(module, api, state, name, plan, distribution, datacenter, lino
instances.append(instance)
elif state in ('stopped'):
for arg in ('name', 'linode_id'):
if not eval(arg):
for arg in (name, linode_id):
if not arg:
module.fail_json(msg='%s is required for active state' % arg)
if not servers:
@ -408,8 +408,8 @@ def linodeServers(module, api, state, name, plan, distribution, datacenter, lino
instances.append(instance)
elif state in ('restarted'):
for arg in ('name', 'linode_id'):
if not eval(arg):
for arg in (name, linode_id):
if not arg:
module.fail_json(msg='%s is required for active state' % arg)
if not servers: