Implement fcntl syscalls
#1893
Implement fcntl syscalls
#1893
Conversation
src/core/IronPython.Modules/fcntl.cs
Outdated
| } | ||
|
|
||
| int argSize = arg.Count; | ||
| IntPtr ptr = Marshal.AllocHGlobal(argSize); |
There was a problem hiding this comment.
AllocHGlobal is one of those methods .NET doesn't want us using. There's a blurb about it in the docs.
There was a problem hiding this comment.
It's not really necessary. CPython uses 1024 bytes on the stack. I could do the same with stackalloc, but it looks to me like a lot of space to put on the stack. Or simply pin a byte array.
| @@ -27,6 +35,91 @@ a file object. | |||
| """; | |||
|
|
|||
|
|
|||
| #region fcntl | |||
|
|
|||
| [LightThrowing] | |||
There was a problem hiding this comment.
Is the light throwing path expected to occur in normal use? If not then I would consider making a regular exception.
Similar question for the other light throwing methods below.
There was a problem hiding this comment.
It depends on the command but yes, sometimes. Those errors are very inexpensive to return so the usage pattern for some commands is simply try/error. For example handling errors EAGAIN, EWOULDBLOCK., EACCES. Having LightThrowing makes it relatively inexpensive here too. Nota bene the code still throws regular exceptions for programming errors like invalid value. That's intentional so that the errors do not go unnoticed when these calls are done from C#.
|
|
||
| #region flock | ||
|
|
||
| [DllImport("libc", SetLastError = true, EntryPoint = "flock")] |
There was a problem hiding this comment.
Wonder what happens on Alpine. 😉
There was a problem hiding this comment.
It works! However, Mono.Unix doesn't load, missing dependency ld-linux-aarch64.so.1, which makes ipy pretty unusable. Even when I installed extra package gcompat (GNU C Library compatibility layer for musl, providing ld), Mono.Unix was still failing with __sprintf_chk: symbol not found.
There was a problem hiding this comment.
I guess my comment was more of a joke, but still unfortunate that Mono.Unix doesn't work on it. 😢
|
For whatever reason github mobile app wouldn't let me add a comment along with my review. My points are just technicalities and not blockers on the PR. I did not hit the approve button because tests are failing on Windows for whatever reason. |
|
The tests are failing not because of the |
|
This seems not to be an IronPython-specific problem, I am getting the same behaviour with CPython 3.4.4. Skipping the test on Windows looks like the right choice. |
|
It is an IronPython-specific problem after all. The test should be skipped if it raises |
This PR implements the main
fcntl, plusflockandlockfand enablestest_fcntlfrom the StdLib.ioctlremains still a mock.The signature of syscal
fcntlis variadic, but luckily this call is supported by Mono.Unix. It means that there is the extra translation step from the given fcntl command code to Mono's enum, and then back to the actual OS code. This step will also filter out (as unsupported) all commands that were not defined yet by the OS at the time Mono.Unix was built. For one thing, I missF_GETPATH, which was added in Python 3.9. It is only on macOS, but on Linux one can get the path name from a descriptor byreadlink /dev/fd/«fileno». Well, maybe I will write a workaround for this one at some point.The calls
flockandlockfare not supported by Mono.Unix, but their signature is simple enough that the P/Invoke prototypes are not a big deal. With them in place it is finally be possible to fix SQLite on macOS (and improve on Linux).