On Sun, Jul 15, 2007 at 12:21:52AM +0200, Joerg Sonnenberger wrote:
> On Fri, Jul 13, 2007 at 01:33:35AM +0000, Arnaud Lacombe wrote:
> > Still about firmload(9), it is normal to have firmware_get_size() returning
> > an `off_t' argument, which is then passed to firmware_read() casted to
> > `size_t'.  Then firmware_read() calls vn_rdwr() where `len' is an `int' to
> > be finaly used to set the `iov_len' and `uio_resid' fields of `struct iovec'
> > which are ...  `size_t'.
>
> Changing of off_t to size_t is bogus, the former is signed.
well, the use of a signed type is a bug in firmload(9):

% grep va_size /usr/include/sys/vnode.h
        u_quad_t        va_size;        /* file size in bytes */

`va_size' is unsigned, so why return a signed type ?

> For file size, off_t is also the natural argument. How do you want to
> handle/detect that a 4GB file is copied to the firmware directory otherwise?
Do you know a lot of firmware that are 4GB big ? Common firmware are about
100kB, it's more than 10^7 times smaller than the 4GB limit. And even if such
firmware exist one day, you would need to have 4GB of kernel memory to load it
directly from the filesystem in 1 vn_rdwr() call.

> I don't know who else uses vn_rdwr, there might be surprises lurking in as
> well.
>
from a quick grep, autoconf (for x86), vnd, kloader, ext2fs, ffs snapshot, ndis,
and few file in compat code.
 
 - Arnaud