[issue8603] Create a bytes version of os.environ and getenvb()

Marc-Andre Lemburg report at bugs.python.org
Mon May 3 15:55:14 CEST 2010


Marc-Andre Lemburg <mal at egenix.com> added the comment:

A view comments on the patch:

+    def __init__(self, data, encodekey, decodekey, encodevalue, decodevalue, putenv, unsetenv):

As general guideline: When adding new parameter, please add them to the
end of the parameter list and preferably with a default argument in order
to not break the API.

Doesn't matter much in this case, since _Environ is only used internally,
but it's still good practice.

--

+data = {}
+for key, value in environ.items():
+    data[_keymap(key)] = fsencode(value)

Please put such init code into a function or make sure that the global
module space is not polluted with temporary variables such as data,
key, value.

--

+    # bytes environ
+    environb = _Environ(data, _keymap, fsencode, fsencode, fsencode, _putenv, _unsetenv)

This looks wrong even though it will work, but that's only a
side-effect of how fsencode is coded and that's not how the
stdlib should be coded (see below).

--

+    def fsencode(value):
+        """
+        unicode to file system
+        """
+        if isinstance(value, bytes):
+            return value
+        else:
+            return value.encode(sys.getfilesystemencoding(), 'surrogateescape')

The function should not accept bytes as input or at least
document this pass-through behavior, leaving the user to decide
whether that's wanted or not.

--

The patch is also missing code which keeps the two dictionaries in
sync. If os.environ changes, os.environb would have to change as
well.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue8603>
_______________________________________


More information about the Python-bugs-list mailing list