At 10:07 AM +0200 2001-06-27, Martin Wilck wrote:
>On Tue, 26 Jun 2001, Jonathan Lundell wrote:
>
>> I use the hack myself, to implement a record-oriented file where the
>> file position is a record number. I could probably live with
>> PAGE_SIZE, but the current hack works fine with start bigger than
>> that, and it's possible that someone counts on it.
>
>Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
>below).
>Unless I'm mislead, legitimate values of "start" as a pointer are always
>larger than that, and I can hardly imagin e a case where the "unsigned
>int" value of start must be greater than PAGE_OFFSET.
PAGE_OFFSET definitely works for me, but a quick scan of the headers
suggests that non-sun3 m68k builds define PAGE_OFFSET as 0, as does
s390.
Maybe you want max(PAGE_SIZE, PAGE_OFFSET).
>I insist that relying on the comparison of two pointers is the wrong
>thing. If (as you suggest) the major use of "start" has migrated from the
>original intention to that of the "hack", this should be reflected
>in the interface by making the "start" parameter to read_proc ()
>an unsigned long. Everything else is misleading and error-prone.
>For now, "start" is a char* and should be treated as such.
That's the hack, though. Rusty should chime in, but the implicit
restriction on start in the original hack (by the time we get to the
test we're talking about) is that it's either a pointer of the form
page+offset, where offset < PAGE_SIZE, or it's a (relatively) small
file offset.
That's a reasonable assumption given that the procedure is
dynamically allocating page. After all, why would you allocate the
buffer and then not use it?
Sure, the overloading is self-admittedly hacky, but (again I assume)
the motivation was to avoid breaking the clients, many of which are
not in the kernel.org tree. Your proposed change overloads a third
interpretation on start, namely an arbitrary pointer, outside the
page allocation.
> > But if you're allocating your own buffer, you'd probably be better
>> off writing your own file ops, and not using the default
>> proc_file_read() at all. At the very least you'd save a redundant
>> __get_free_page/free_page pair.
>
>That's right, but nevertheless (repeat) comparing "start" and "page" is
>wrong.
Not given the implied restriction that, if start is a pointer at all,
it's a pointer within page's allocation. And after all, PAGE_OFFSET
is effectively a pointer.
-- /Jonathan Lundell. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Jun 30 2001 - 21:00:16 EST