-
-
Notifications
You must be signed in to change notification settings - Fork 34.5k
bpo-28009: Fix uuid.uuid1() and uuid.get_node() on AIX #8672
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
Changes from 1 commit
4ed41e9
ba47933
c6029a4
7e1874d
6a5bccb
a98309b
53ef750
6fd5f8e
3bb1c08
1af8e3d
d3eaab9
3469a01
399e8ec
6e2a9bf
bb3a460
db6767f
25d3ef1
fa9b43b
4a0c8f0
6463707
a0ef760
095e221
ec4c0e8
70a45f0
b1b4952
8f0687a
4756670
c55714a
10f272e
27b6c32
33969b9
7a57734
4628cea
d2830a2
0688727
ff1ee20
083e9c6
30cd017
27a972b
543e66d
588fda7
28f7a01
6fc2129
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -678,7 +678,7 @@ class BaseTestInternals: | |
| # and strings shorter than 17 bytes (no leading 0) | ||
| @unittest.skipUnless(_AIX, 'requires AIX') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the skip logic changes that you extracted out to other PRs, a better check here will be
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That also suggests abstracting out a marker attribute for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You (or I) resolved this. Both marker attributes are defined, and rather than skip, are assigned to the mock tests. If I have missed something - I'll get on it, if not - please consider approving the changes, Thx.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that this is a valid test anywhere that those two conditions are true, so rather than testing for "AIX_" here, you can just test for the MAC notation conventions that the test cares about. |
||
| # key is on lineX, value is on lineX+1 aka 'nextline' | ||
| def test_find_mac_nextline(self): | ||
| def test_find_mac_nextlines(self): | ||
| data = '''\ | ||
| Name Mtu Network Address Ipkts Ierrs Opkts Oerrs Coll | ||
| en0 1500 link#2 fe.ad.c.1.23.4 1714807956 0 711348489 0 0 | ||
|
|
@@ -695,11 +695,11 @@ def test_find_mac_nextline(self): | |
| return_value='/usr/bin/netstat'): | ||
| with mock.patch.object(subprocess, 'Popen', | ||
| return_value=popen): | ||
| mac = self.uuid._find_mac_nextline( | ||
| mac = self.uuid._find_mac_nextlines( | ||
| command='netstat', | ||
| args='-ia', | ||
| hw_identifiers=b'Address', | ||
| get_index=lambda x: x, | ||
| f_index=lambda x: x, | ||
| ) | ||
|
|
||
| self.assertEqual(mac, 0xfead0c012304) | ||
|
aixtools marked this conversation as resolved.
|
||
|
|
@@ -724,7 +724,7 @@ def test_find_mac_inline(self): | |
| command='ifconfig', | ||
| args='', | ||
| hw_identifiers=[b'hwaddr'], | ||
| get_index=lambda x: x + 1, | ||
| f_index=lambda x: x + 1, | ||
| ) | ||
|
|
||
| self.assertEqual(mac, 0x1234567890ab) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,6 @@ | |
|
|
||
| __author__ = 'Ka-Ping Yee <ping@zesty.ca>' | ||
|
|
||
| ======= | ||
| # The recognized platforms - known behaviors | ||
| _AIX = platform.system() == 'AIX' | ||
| _DARWIN = platform.system() == 'Darwin' | ||
|
|
@@ -386,8 +385,14 @@ def _popen(command, *args): | |
| def _is_universal(mac): | ||
| return not (mac & (1 << 41)) | ||
|
|
||
|
|
||
| # In the next two fucnctions: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I understand the general approach here, and it makes sense. These two functions need a better explanation of what they do, though—the comments here aren't enough for a reader to understand how they're intended to work, and the argument names Here's an attempt at some better names and a docstring: Given that Another concern I have is that the logic about what looks like a MAC address is mixed in with the logic about where to find it. It would be ideal to factor it out, like this: Then: etc.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. |
||
| # command: name of command to run | ||
| # args: arguments passed to command | ||
| # hw_identifers: keywords used to locate a value | ||
| # f_index: lambda function to modify, if needed, an index value | ||
| # keyword and value are on the same line aka 'inline' | ||
| def _find_mac_inline(command, args, hw_identifiers, get_index): | ||
| def _find_mac_inline(command, args, hw_identifiers, f_index): | ||
| first_local_mac = None | ||
| try: | ||
| proc = _popen(command, *args.split()) | ||
|
|
@@ -399,7 +404,7 @@ def _find_mac_inline(command, args, hw_identifiers, get_index): | |
| for i in range(len(words)): | ||
| if words[i] in hw_identifiers: | ||
| try: | ||
| word = words[get_index(i)] | ||
| word = words[f_index(i)] | ||
| mac = int(word.replace(_MAC_DELIM, b''), 16) | ||
| if _is_universal(mac): | ||
| return mac | ||
|
|
@@ -415,32 +420,37 @@ def _find_mac_inline(command, args, hw_identifiers, get_index): | |
| pass | ||
| return first_local_mac or None | ||
|
|
||
| # value is on the line following the keyword - aka 'nextline' | ||
| def _find_mac_nextline(command, args, hw_identifiers, get_index): | ||
| # Keyword is only on firstline - values on remaining lines | ||
| def _find_mac_nextlines(command, args, hw_identifiers, f_index): | ||
| first_local_mac = None | ||
| mac = None | ||
| try: | ||
| proc = _popen(command, *args.split()) | ||
| if not proc: | ||
| return None | ||
| with proc: | ||
| words = proc.stdout.readline().rstrip().split() | ||
| keywords = proc.stdout.readline().rstrip().split() | ||
| try: | ||
| i = words.index(hw_identifiers) | ||
| i = keywords.index(hw_identifiers) | ||
| except ValueError: | ||
| return None | ||
| # we have the index (i) into the data that follows | ||
| for line in proc.stdout: | ||
| try: | ||
| words = line.rstrip().split() | ||
| word = words[get_index(i)] | ||
| if len(word) == 17 and word.count(_MAC_DELIM) == 5: | ||
| mac = int(word.replace(_MAC_DELIM, b''), 16) | ||
| elif _AIX and word.count(_MAC_DELIM) == 5: | ||
| values = line.rstrip().split() | ||
| value = values[f_index(i)] | ||
| if len(value) == 17 and value.count(_MAC_DELIM) == 5: | ||
| mac = int(value.replace(_MAC_DELIM, b''), 16) | ||
| # (Only) on AIX the macaddr value given is not prefixed by 0, e.g. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the revised logic,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| # en0 1500 link#2 fa.bc.de.f7.62.4 110854824 0 160133733 0 0 | ||
| # not | ||
| # en0 1500 link#2 fa.bc.de.f7.62.04 110854824 0 160133733 0 0 | ||
| elif _AIX and value.count(_MAC_DELIM) == 5: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this can change to |
||
| # the extracted hex string is not a 12 hex digit | ||
| # string, so add the fields piece by piece | ||
| if len(word) < 17 and len(word) >= 11: | ||
| if len(value) < 17 and len(value) >= 11: | ||
| mac = 0 | ||
| fields = word.split(_MAC_DELIM) | ||
| fields = value.split(_MAC_DELIM) | ||
| for hex in fields: | ||
| mac <<= 8 | ||
| mac += int(hex, 16) | ||
|
|
@@ -458,6 +468,9 @@ def _find_mac_nextline(command, args, hw_identifiers, get_index): | |
| pass | ||
| return first_local_mac or None | ||
|
|
||
|
|
||
| # The following functions call external programs to 'get' a macaddr value to | ||
| # be used as basis for an uuid | ||
| def _ifconfig_getnode(): | ||
| """Get the hardware address on Unix by running ifconfig.""" | ||
| # This works on Linux ('' or '-a'), Tru64 ('-av'), but not all Unixes. | ||
|
|
@@ -510,7 +523,7 @@ def _lanscan_getnode(): | |
| def _netstat_getnode(): | ||
| """Get the hardware address on Unix by running netstat.""" | ||
| # This works on AIX and might work on Tru64 UNIX. | ||
| mac = _find_mac_nextline('netstat', '-ia', b'Address', lambda i: i) | ||
| mac = _find_mac_nextlines('netstat', '-ia', b'Address', lambda i: i) | ||
| return mac or None | ||
|
|
||
| def _ipconfig_getnode(): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| Fix uuid.uuid1() and uuid.get_node() on AIX | ||
| Fix uuid.getnode() on AIX - to use proper MAC_DELIM character | ||
|
aixtools marked this conversation as resolved.
Outdated
|
||
Uh oh!
There was an error while loading. Please reload this page.