I am looking for suggestions of improving this function to check whether a memory address is appropriate or not.
def isweird(addr, cpu_name):
"""Check if a memory address is weird
Args:
addr (str): a memory address
cpu_name (str): cpu name
Returns:
bool: True if the address is weird
"""
if not isinstance(addr, six.string_types):
raise Exception('The memory address must be a string.')
if addr == '0x0':
return True
addr = addr.lower()
# Strip leading zeroes
addr = addr[2:].lstrip('0')
if utils.is64(cpu_name):
if len(addr) <= 8:
val = int(addr, 16)
if val <= (1 << 16): # val <= 0xffff (ie: first 64k)
return True
elif addr.startswith('ffffffff'):
addr = addr[8:] # 8 == len('ffffffff')
val = int(addr, 16)
if val >= ((1 << 32) - (1 << 16)): # val >= 0xffffffffffff0000 (ie: last 64k)
return True
else:
val = int(addr, 16)
if val <= 1 << 16: # val <= 0xffff (ie: first 64k)
return True
if val >= ((1 << 32) - (1 << 16)): # val >= 0xffff0000 (ie: last 64k)
return True
return False
2 Answers 2
In this code:
if len(addr) <= 8: val = int(addr, 16) if val <= (1 << 16): # val <= 0xffff (ie: first 64k) return True elif addr.startswith('ffffffff'): addr = addr[8:] # 8 == len('ffffffff') val = int(addr, 16) if val >= ((1 << 32) - (1 << 16)): # val >= 0xffffffffffff0000 (ie: last 64k) return True
You can replace the inner if
statements with return
statements:
if len(addr) <= 8:
val = int(addr, 16)
return val <= (1 << 16) # val <= 0xffff (ie: first 64k)
elif addr.startswith('ffffffff'):
addr = addr[8:] # 8 == len('ffffffff')
val = int(addr, 16)
return val >= ((1 << 32) - (1 << 16)) # val >= 0xffffffffffff0000 (ie: last 64k)
This is possible because if these conditions are false,
the execution will reach the return False
at the end of the function.
The reason to rewrite this way is so that the reader doesn't have to follow the execution path until the end of the function,
he can know on these lines that no further statements will be executed,
the return value is already decided at these points.
Some of the comments are incorrect and misleading:
if val <= (1 << 16): # val <= 0xffff (ie: first 64k) if val >= ((1 << 32) - (1 << 16)): # val >= 0xffffffffffff0000 (ie: last 64k)
1 << 16
is 0x10000
, not 0xffff
.
(1 << 32) - (1 << 16)
is 0xffff0000
, not 0xffffffffffff0000
.
Following the naming conventions of Python,
the function isweird
should be named is_weird
instead.
Don't use binary operators when you can use numbers. Take:
if val <= (1 << 16): # val <= 0xffff (ie: first 64k)
This can just be:
if val <= 0xffff:
This allows you to remove the comments, whilst keeping clarity for users to understand what numbers you are using.
-
\$\begingroup\$ "boolean operators" I guess you meant "binary operators" \$\endgroup\$Caridorc– Caridorc2016年08月31日 18:39:58 +00:00Commented Aug 31, 2016 at 18:39
-
\$\begingroup\$ Minor correction, 1<<16 == 0x10000 \$\endgroup\$user650881– user6508812017年01月11日 05:00:02 +00:00Commented Jan 11, 2017 at 5:00