Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Dec 2002 14:58:39 +1100 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        Julian Elischer <julian@elischer.org>, Nate Lawson <nate@root.org>, Julian Elischer <julian@FreeBSD.org>, <cvs-all@FreeBSD.org>, <cvs-committers@FreeBSD.org>
Subject:   Re: cvs commit: src/sys/i386/i386 dump_machdep.c
Message-ID:  <20021218143915.P23372-100000@gamplex.bde.org>
In-Reply-To: <200212170108.gBH18V4r082434@apollo.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 16 Dec 2002, Matthew Dillon wrote:

> :On Mon, 16 Dec 2002, Matthew Dillon wrote:
> :
> :>     Hmm.  Wouldn't this be easier if the test were done after
> :>     calculating dumplo?  e.g.
> :>
> :> :>  	dumplo = di->mediaoffset + di->mediasize - Maxmem * (off_t)PAGE_SIZE;
> :> :>  	dumplo -= sizeof kdh * 2;
> :>
> :
> :no because then you'd have to check fopr dumplo being -ve
> :and I'm of the opinion that blocknumbers should always be unsigned (*)
> :so if this were ever "corrected" to be unsigned, you'd
> :have to check for an overflow which is difficult.

Indeed.  But some reorganization to compute the size only once would be
useful.  E.g.:

	size = sizeof(kdh) + Maxmem * (off_t)PAGE_SIZE + sizeof(kdh);
	size_including_headroom = 64 * 1024 + size;
	/* Use previous variable in space check and error message about it. */
 	dumplo = di->mediaoffset + di->mediasize - size;

> :(*) there are many bugs showing up at the moment as disks start to
> :get NBLK > 2^31 blocks on them that wouldn't happen if they were
> :unsigned..

These bugs are features.  If block addresses were unsigned 32 bits then
they would wrap around at 2^32 and probably cause writes to the wrong
blocks, but if block addresses are signed 32 bits then they overflow at
2^31 and on non-exotic machines cause negative block numbers which are
obviously bogus so they cause harmless write errors.

> :I have 3 such devices each with >1TB.
> :If you want to see things explode, just try run sysinstall on them.
>
>     Isn't it using off_t there?  64 bit offsets?

As pointed out before, off_t is for file offsets and is bogus for block
offsets.  off_t is specified to be signed by POSIX, mainly so that it
can be used for seeks backwards.  Since off_t is signed, using it may
reduce overflow bugs, but only in contexts where it is not mixed with
sizeof().

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021218143915.P23372-100000>