-
Notifications
You must be signed in to change notification settings - Fork 22
Fix Python 3.6.0 #5
base: master
Are you sure you want to change the base?
Conversation
usbinfo/darwin.py
Outdated
@@ -30,6 +31,38 @@ | |||
OSX_VERSION_MICRO_INT = [int(part) for part in OSX_VERSION_STR.split('.')] | |||
|
|||
|
|||
def _sanitize_xml(data): | |||
pattern = re.compile(r'^(\s*?\<string\>)(.*?)(\<\/string\>.*?)$') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be compiled once instead of re-compiling on every function call?
@@ -38,6 +71,10 @@ def _ioreg(nodename): | |||
"""Run ioreg command on specific node name""" | |||
cmd = ['ioreg', '-a', '-l', '-r', '-n', nodename] | |||
output = subprocess.check_output(cmd) | |||
# ST-Link devices (and possibly others?) erroneously store binary data | |||
# in the <string> serial number, which causes plistlib to blow up. | |||
# This will convert that to hex and preserve contents otherwise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add this comment to the function definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
to crash. | ||
|
||
Returns the same document, with any mis-encoded <string> values converted | ||
to ascii hex.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should follow the indentation style for other docstrings.
|
||
data = data.decode('utf-8') | ||
|
||
for i, line in enumerate(data.split('\n')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to enumerate if you're not using the index.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use splitlines().
to ascii hex.""" | ||
output = [] | ||
|
||
data = data.decode('utf-8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are mutating data at the caller here. You could just encode it ad the for loop itself.
output.append(line) | ||
output = '\n'.join([line for line in output]) | ||
|
||
return output.encode('utf-8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this redundant?
output.append(line) | ||
else: | ||
output.append(line) | ||
output = '\n'.join([line for line in output]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can simply be:
return '\n'.join(output)
data = data.decode('utf-8') | ||
|
||
for i, line in enumerate(data.split('\n')): | ||
chunk = line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary assignment not necessary.
for byte in byte_list: | ||
if byte < 32: | ||
needs_patch = True | ||
if needs_patch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this all be reduced to just
if any([ord(byte) not in range(32, 128) for byte in middle]):
...
|
||
sanitize_pattern = re.compile(r'^(\s*?\<string\>)(.*?)(\<\/string\>.*?)$') | ||
|
||
def _sanitize_xml(data): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth investigating doing all of this in an XML library. This function is assuming that ioreg
will always return data in a specific format.
Python 3.6.0's plistlib has removed readPlistFromString in favor of loads. There isn't a backwards compatible upgrade path, so feature detection was necessary instead.
This patch fixes Python 3.6.0 by trying plistlib.loads when AttributeError is thrown from attempting to call plistlib.readPlistFromString.
Additionally, dict.iteritems() is deprecated in favor of dict.items() in Python 3. This also works in Python 2.7.
Tested both Python 2.7 (standard install) and Python 3.6.0 (Anaconda) on macOS Sierra 10.12.4 (16E195).