Thread (20 messages) 20 messages, 6 authors, 2019-12-18

Re: [PATCH 4/6] iomap: add struct iomap_ctx

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2019-12-17 20:26:46
Also in: linux-fsdevel, linux-mm

On Tue, Dec 17, 2019 at 11:39 AM Linus Torvalds
[off-list ref] wrote:
'loff_t length' is not right.
Looking around, it does seem to get used that way. Too much, though.
quoted
+       loff_t pos = data->pos;
+       loff_t length = pos + data->len;
And WTH is that? "pos + data->len" is not "length", that's end. And this:
quoted
        loff_t end = pos + length, done = 0;
What? Now 'end' is 'pos+length', which is 'pos+pos+data->len'.
But this is unrelated to the crazy types. That just can't bve right.
Is there some reason for this horrible case of "let's allow 64-bit sizes?"

Because even if there is, it shouldn't be "loff_t". That's an
_offset_. Not a length.
We do seem to have a lot of these across filesystems. And a lot of
confusion. Most of the IO reoutines clearly take or return a size_t
(returning ssize_t) as the IO size. And then you have the
zeroing/truncation stuff that tends to take loff_t. Which still smells
wrong, and s64 would look like a better case, but whatever.

The "iomap_zero_range() for truncate" case really does seem to need a
64-bit value, because people do the difference of two loff_t's for it.
In fact, it almost looks like that function should take a "start ,
end" pair, which would make loff_t be the _right_ thing.

Because "length" really is just (a positive) size_t normally.

                Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help