Neil Schemenauer <nas-pyt...@arctrix.com> added the comment:
I didn't finish reviewing completely yet but here are some comments.
I think the random filename generation can be pulled out of
_posixshmem. Make it require that a filename is passed into it.
That moves a fair bit of complexity out of C and into Python. In
the Python module, I suggest that you could use secrets.token_hex()
to build the filename. Put the loop that retries because of name
collisions in the Python module as well.
If you like, I can try to make a patch that does the above.
When looking at at how the Python code would handle a name collision,
I see this code:
+ switch (errno) {
+ case EACCES:
+ PyErr_Format(pPermissionsException,
+ "No permission to %s this segment",
+ (flags & O_TRUNC) ? "truncate" : "access"
+ );
+ break;
+
+ case EEXIST:
+ PyErr_SetString(pExistentialException,
+ "Shared memory with the specified name already
exists");
+ break;
+
+ case ENOENT:
+ PyErr_SetString(pExistentialException,
+ "No shared memory exists with the specified
name");
+ break;
+
+ case EINVAL:
+ PyErr_SetString(PyExc_ValueError, "Invalid parameter(s)");
+ break;
+
+ case EMFILE:
+ PyErr_SetString(PyExc_OSError,
+ "This process already has the maximum number
of files open");
+ break;
+
+ case ENFILE:
+ PyErr_SetString(PyExc_OSError,
+ "The system limit on the total number of open
files has been reached");
+ break;
+
+ case ENAMETOOLONG:
+ PyErr_SetString(PyExc_ValueError,
+ "The name is too long");
+ break;
+
+ default:
+ PyErr_SetFromErrno(PyExc_OSError);
+ break;
+ }
I think it might be cleaner just to make it:
PyErr_SetFromErrno(PyExc_OSError);
Then, if you need a customized exception or message, you can
re-raise inside the Python code. To me, it seems simpler and more
direct to just preserve the errno and always raise OSError. Changing things
to ValueError means that callers need to look at the message text to
differentiate between some errno values.
Is it the case that _posixshmem started life as a module that would
be used directly and not something hidden by another layer of
abstraction? If so, having these customized exceptions and having
it do the filename generation itself makes sense. However, it is an
internal implementation detail of shared_memory.py, I think it
should be simplified to do only what is needed. E.g. a thin layer
of the system calls.
----------
nosy: +nascheme
_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue35813>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
