[Pitch] Add non-throwing FileDescriptor.currentOffset helper #50
Conversation
Provided the file descriptor is still valid, accessing the current offset shouldn't fail.
| @_alwaysEmitIntoClient | ||
| public var currentOffset: Int64 { | ||
| try! _seek(offset: 0, from: .current).get() | ||
| } |
There was a problem hiding this comment.
What about for files that have no concept of position?
[ESPIPE] Fildes is associated with a pipe, socket, or FIFO.
That being said, I do like the idea of a very simple AEIC helper.
There was a problem hiding this comment.
Great point, I must have missed that one in my first pass. Here’s a review of the error codes on Darwin and how we might handle each:
EBADF: Implies that the instance was constructed with an invalid descriptor usinginit(rawValue:), or is being called afterclose(), or something else. I think it makes sense to trap here.EINVAL: The seek location andwhencearen’t provided by the client. This would presumably be a bug in System, and should trap.ENXIO: Only applies toSEEK_DATAandSEEK_HOLE.EOVERFLOW: File is larger than would fit in anInt64. Should probably trap?ESPIPE: No concept of a current location. This seems much more likely than the other cases, and so a trap here might be more surprising to the caller.
According to https://man7.org/linux/man-pages/man2/lseek.2.html, calling lseek(2) on a terminal device will fail with ESPIPE. I just tested this on Darwin with stdin, and it doesn’t fail, although the return value is probably meaningless.
My first inclination was to change the return type to Int64?. We could still trap for the above documented cases, but on ESPIPE return nil. It’s already up to the user to understand what kind of file descriptor they’re working with, which they could use to decide whether to force unwrap. However, a given file descriptor is either always going to return a valid offset, or always return nil, and being required to force unwrap when you know you have a regular file is unfortunate (although still far more convenient than try! fd.seek(offset: 0, from: .current)).
Depending on System’s source compatibility story, a second option is a throwing accessor, resulting in try fd.currentOffset. This doesn’t discard any information, while providing a convenient shorthand.
I think it might boil down to this: one has to know whether this method is valid to call on a given file descriptor already. Given the API options I’ve described so far, all of the following would trap: fd.currentOffset, fd.currentOffset! or try! fd.currentOffset.
If we can require Swift 5.5, I think I’m most in favour of the last one. I don’t think the optional version of the API is enough of an improvement over the current patch. If requiring Swift 5.5 isn’t an option, maybe we could make it a method instead, or perhaps guard it with an #if compiler() directive?
There was a problem hiding this comment.
I'm also inclined towards the throwing computed var. I'll have to check how to deal with the required tools versions.
Provided the file descriptor is still valid, accessing the current offset shouldn't fail.
It took me a bit of hunting around to figure out how to access the current offset in a file. I could see this being too trivial to add a helper for, however, after reading the man page for
lseek(2), it seemed like the operation shouldn't be able to fail for valid file descriptors, and so there's value in providing a non-throwing way to achieve this.Would love to hear your feedback. Thanks!