Add typing for most variables/returns

[C0rn3j]

I was not a 100% on typing t and size, so I left them alone, rest is typed now.

[vpelletier]

To give more context about t/size: the inspiration is https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/ioctl.h , and I now see the latter was renamed in torvalds/linux@6d89ead .

The intention is:

• allow types supported by ctypes.sizeof, as an equivalent to C source's sizeof use
• add support for some native python types for usage convenience (at the expense of dynamic type-checking [EDIT: meaning isinstance]), when they make sense for ioctl use (array because it forces a regular item size, similarly for struct, and memoryview/bytearray because they allow zerocopy buffers)

Maybe these types have a common typing ancestor ? Maybe they all implement the buffer protocol (I did not check if it is the case in practice, but that would make some sense) ?

Another type I know of which could make sense is mmap.mmap.

[vpelletier]

Maybe they all implement the buffer protocol (I did not check if it is the case in practice, but that would make some sense) ?

I see 3.12 introduced collections.abc.Buffer for this purpose.

In the context of an out-of-stdlib, this feels too recent to require it (both as type hint and more generally to rely on the __buffer__/__release_buffer__ API to simplify the if isinstance(...)/elif isinstance(...)).

On the context of an stdlib candidate, I guess it would be desirable though.

So I feel there is a need to have two branches in this repository:

• one to remain as an off-stdlib module which can be depended upon when running against older versions of the stdlib, which would retain backward compatibility with older stdlib versions
• one to prepare a candidate for inclusion in the stdlib, which AFAIK does not need to care about backward compatibility and would certainly benefit from such code simplification (especially removal of such unpytonic isinstance checks)

Both should try to remain as close as possible (...which in practice should be very easy, this repository is so quiet).

Does this make sense ?

[C0rn3j]

I wanted to add a short description on the first line to this function to use PEP 257 recommendations, would you have an idea?

The rest already had it or were short enough to be able to be used.

def IOC(dir: int, type: int, nr: int, size) -> int:
    """
    dir
        One of IOC_NONE, IOC_WRITE, IOC_READ, or IOC_READ|IOC_WRITE.
        Direction is from the application's point of view, not kernel's.
    size (14-bits unsigned integer)
        Size of the buffer passed to ioctl's "arg" argument.
    """

[C0rn3j]

3.12 ... this feels too recent to require it

While <3.12 will be here with us for 2 more years, I don't believe it's a problem, people needing older Python versions can simply use the older version of the library, it's not like the typing additions are going to matter that much to people (otherwise someone would have contributed them by now).

IMO, no need to split the library in two - but again, your call

[vpelletier]

Sorry for the delay.

I wanted to add a short description on the first line to this function to use PEP 257 recommendations,

Good idea, thanks.

would you have an idea?

Maybe something like Produce IOCTL command number from raw components, echoing the module's docstring. Maybe followed by Consider using IO, IOR, IOW or IORW or Called by IO, IOR, IOW and IORW as ideally those should do the job.

people needing older Python versions can simply use the older version of the library, it's not like the typing additions are going to matter that much to people

Good point. Let's not split the repository.

[vpelletier]

I see 3.12 introduced collections.abc.Buffer for this purpose.

Here is the general direction in which I intend to go:

diff --git a/ioctl_opt/__init__.py b/ioctl_opt/__init__.py
index aa2006f..d234991 100644
--- a/ioctl_opt/__init__.py
+++ b/ioctl_opt/__init__.py
@@ -26,6 +26,7 @@ Common parameter meanings:
         Driver-imposed ioctl function number.
 """
 import array
+from collections.abc import Buffer
 import ctypes
 import struct
 from typing import Final
@@ -69,14 +70,11 @@ def IOC(dir: int, type: int, nr: int, size) -> int:
     assert size <= _IOC_SIZEMASK, size
     return (dir << _IOC_DIRSHIFT) | (type << _IOC_TYPESHIFT) | (nr << _IOC_NRSHIFT) | (size << _IOC_SIZESHIFT)

-def IOC_TYPECHECK(t) -> int:
+def IOC_TYPECHECK(t: Buffer | ctypes._CData | type[ctypes._CData]) -> int:
     """Returns the size of given type, and check its suitability for use in an ioctl command number."""
-    if isinstance(t, (memoryview, bytearray)):
-        size = len(t)
-    elif isinstance(t, struct.Struct):
-        size = t.size
-    elif isinstance(t, array.array):
-        size = t.itemsize * len(t)
+    if isinstance(t, Buffer):
+        with memoryview(t) as t_view:
+            size = t_view.nbytes
     else:
         size = ctypes.sizeof(t)
     assert size <= _IOC_SIZEMASK, size
diff --git a/pyproject.toml b/pyproject.toml
index 09e75bb..a1fbcba 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,7 +6,7 @@
        authors = [{name = "Vincent Pelletier", email = "[email protected]"}]
        license = { text = "GPL-2.0-or-later" }
        keywords = ["ioctl"]
-       requires-python = ">=3.10"
+       requires-python = ">=3.12"
        classifiers = [
                "Intended Audience :: Information Technology",
                "License :: OSI Approved :: GNU General Public License (GPL)",

Of course, it is bad to have to reach for ctypes' underware - not to mention this class is not actually reachable in this way, though it can be reached via its public subclasses - like ctypes.c_int32.__bases__[0].__bases__[0]. An alternative would be to annotate with Buffer | type[Buffer] but this seems too vague: IOC_TYPECHECK(mmap.mmap) is not intended to work. The only acceptable types must have a byte size: either an instance of a buffer type, or something ctypes.sizeof can work on (ctypes.Structure subclass, typically, but it could also be a subclass of any ctypes.c_*).

Can this be expressed in a nicer way ?

Side note: while implementing the above I realize there is a bug in the current implementation when an instance of memoryview or bytearray (or of their subclasses) is provided and the item size is not one byte. The above patch fixes this.

[C0rn3j]

I've rebased against master and applied the patch + suggestions.

I am unfortunately not sure how I'd improve the typing further.