On Sun, Jul 15, 2007 at 01:35:09AM +0000, Arnaud Lacombe wrote:
> 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):

I just say that you also have to ensure the values are correct if you
want to change that.

> > 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.

Given the growth rate of firmware image...

> > 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.

...and you have audited them that no conversion issues can pop up?

Joerg