SeekFrom with different inner types is confusing (and non-POSIX)

[amosonn]

Currently, SeekFrom has different "inner" type depending on whether we seek from the Start (u64) or from the Current or End position (i64). This of course only makes sense, if we assume that Seek objects can (and should) support a u64 size. This has a few problems (in rising order of importance, IMO):

1. This deviates from POSIX standard for files, which dictates seeking by off_t (http://man7.org/linux/man-pages/man2/lseek.2.html), which is defined to be signed (http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html#tag_13_67), for all SeekFrom-s.
2. You can only access part of the file when seeking from Current or End, which kinda sucks. Not necessarily terrible, but also not the best API.
3. It makes the arithmetic very tricky. See here: Overflow in libstd/io/cursor.rs "seek" returns incorrect error with large positions rust#39631. Of course, this can be remedied, (one possible solution described there), but I think that almost anyone who wants to implement Seek for a custom type (which is not a wrapper of an inner, Seek type) will encounter this problem. That's how I found it - I was at loss how to properly do the arithmetic, looked for a stdlib example, and found it was buggy. The other place I found in stdlib implementing a custom Seek type was in test_buffered_reader_seek_underflow() in libstd/io/buffered.rs, and there they overcame the problem by making the type a "circular" reader. I presume this is because implementing proper arithmetic considering the two different types was too much work (and irrelevant for the test), but of course isn't applicable for many real-life cases.

Generally speaking, I think that it is better to change an API that pushes people to have buggy implementations, especially when the ramifications are quite measly. The reason the bug above is still in stdlib is because nobody cares, because we never really have objects that are this big, which means we might as well use i64 to store our position. For files, this also the standard, so no trouble with the OS interface there.

[sfackler]

We are not going to break backwards compatibility and change the type in this field.

Do you work with seekable structures that are over 9.2 exabytes? How would point 2 be improved if Start and End stored an i64?

[amosonn]

The current form implies that `pos` and `size` in a file should be `u64`. This is not very useful (over `i64`), because no real structure needs this. But if someone implemented some case where this could happen, an object whose "real" size is not related to its "API" size (ex. sparesly allocated memory buf with prespecified size), they are likely to make the same mistake, and then when someone (else) does somehow use the full `u64` (just because they can - in the example above it makes sense), things will break. On the other hand, if `Start` were to hold `i64`, it would be implied that `pos`, as well as `size`, should be `i64`, no usability will really get hurt (the user from above will use the maximal `i64` and will be fine), but no trouble will ensue. In my usecase I get the effective size from outside. It shouldn't be greater than `i64`, but if I don't check this (and why should I?), it might. On Feb 8, 2017 1:17 AM, "Steven Fackler" <[email protected]> wrote: We are not going to break backwards compatibility and change the type in this field. Do you work with seekable structures that are over 9.2 exabytes? How would point 2 be improved if Start and End stored an i64? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1889 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC_zrQBmtW6B3clxgSAcL6Er-kkIa6_qks5raQmIgaJpZM4L6Nm8> .

[amosonn]

What I mean is that if an edge case doesn't matter, it's better to forbid it than to let it break.

…

On Feb 8, 2017 1:36 AM, "Amos Onn" ***@***.***> wrote: The current form implies that `pos` and `size` in a file should be `u64`. This is not very useful (over `i64`), because no real structure needs this. But if someone implemented some case where this could happen, an object whose "real" size is not related to its "API" size (ex. sparesly allocated memory buf with prespecified size), they are likely to make the same mistake, and then when someone (else) does somehow use the full `u64` (just because they can - in the example above it makes sense), things will break. On the other hand, if `Start` were to hold `i64`, it would be implied that `pos`, as well as `size`, should be `i64`, no usability will really get hurt (the user from above will use the maximal `i64` and will be fine), but no trouble will ensue. In my usecase I get the effective size from outside. It shouldn't be greater than `i64`, but if I don't check this (and why should I?), it might. On Feb 8, 2017 1:17 AM, "Steven Fackler" ***@***.***> wrote: We are not going to break backwards compatibility and change the type in this field. Do you work with seekable structures that are over 9.2 exabytes? How would point 2 be improved if Start and End stored an i64? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1889 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC_zrQBmtW6B3clxgSAcL6Er-kkIa6_qks5raQmIgaJpZM4L6Nm8> .

[sfackler]

Again, it is a year and a half too late to make that change.

On the other hand, if Start were to hold i64, it would be implied that
pos, as well as size, should be i64

I don't think that would be the case - off_t is 32 bits in many cases on systems that support files over 2GB and you have to seek around with multiple calls if _FILE_OFFSET_BITS=64 isn't an option.

[amosonn]

Well, "Too late" is definitely a good argument.
Just out of curiosity, how does Rust handle seeking in 32-bit platforms, given that the types in SeekFrom are fixed to 64-bit?
Thanks for your time!

[sfackler]

It uses lseek64.

[Centril]

Seems this was resolved and the libs team is not inclined to do anything here so I'll close this.