bpo-28009: Fix uuid.uuid1() and uuid.get_node() on AIX by aixtools · Pull Request #8672 · python/cpython (original) (raw)

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 hw_identifiers and f_index are quite cryptic.

Here's an attempt at some better names and a docstring:

def _find_mac_near_keyword(command, args, keywords, get_word_index):
    """Searches the output of a command for a MAC address near a keyword.

    Each line of words in the output is lowercased and searched for any of
    the given keywords.  Upon a match, get_word_index is invoked to pick a
    word from the line, given the index of the match (e.g. lambda i: 0 gets the
    first word on the line; lambda i: i - 1 gets the word preceding the keyword)."""

Given that _find_mac_nextlines is supposed to look for words under column headings, there's no need for it to take an f_index function, and the third argument can be named more clearly. Indeed, it would be misleading to call the third argument hw_identifiers, as it's a single string, not a list of strings. Here's a try:

def _find_mac_under_heading(command, args, heading):
    """Looks for a MAC address under a heading in the output of a command.

    The first line of words in the output is searched for the given heading.
    Words at the same word index as the heading in subsequent lines are then
    examined to see if they look like MAC addresses."""

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:

def _find_near_keyword(command, args, keywords, get_word_index):
    """Yields selected nearby words for keywords found in the output of a command.
    ...
        yield _parse_mac(word)
    ...

def _find_mac_under_heading(command, args, heading):
    """Yields words found under a heading in the output of a command.

    The first line of words in the output is searched for the given heading.
    For each subsequent line, the word at the same word index as the heading
    in subsequent lines is then yielded."""
    
def _parse_mac(word):
    """Converts a string to a 48-bit integer if it looks like a MAC address, or returns None."""
    # A MAC address is six bytes, each printed as one or two hex digits.
    # They may be separated by ':' or '.' depending on the platform.
    parts = word.replace('.', ':').split(':')
    if len(parts) == 6 and all(len(part) <= 2 for part in parts):
        hexstr = b''.join(part.rjust(2, b'0') for part in parts)
        try:
            return int(hexstr, 16)
        except ValueError:
            pass

def _select_best_mac(macs):
    """Chooses a MAC from the given iterable, preferring any universally administered
    MAC address over locally administered MAC addresses."""
    local_macs = []
    for mac in macs:
        if _is_universal(mac):
            return mac
        elif mac:
            local_macs.append(mac)
    if local_macs:
        return local_macs[0]

Then:

_select_best_mac(_find_near_keyword('ifconfig', args, keywords, lambda i: i + 1))
_select_best_mac(_find_under_heading('netstat', '-ia', b'Address')

etc.