On 17 Jun 2014 18:22, "Kenneth Westerback" <[email protected]> wrote: > > On 17 June 2014 17:43, Tobias Stoeckmann <[email protected]> wrote: > > On Mon, Jun 16, 2014 at 04:43:02PM -0700, John-Mark Gurney wrote: > >> FreeBSD fixed this by increasing the malloc size: > >> https://svnweb.freebsd.org/changeset/base/r126086 > > > > Which is actually the correct way to do here! > > > > pmp->pm_maxcluster is the largest valid _index_ of pmp->pm_inusemap, > > therefore we must allocate pmp->pm_maxcluster + 1. > > > > The "howmany" in fillinusemap took that into account. Instead of > > writing + 1 - 1, it is skipped. > > > > In msdosfs_mountfs, it's missing... > > > > So I will vote for FreeBSD's commit instead: Introducing howmany > > macro AND doing the same in msdosfs_fat.c to help the next person > > looking at that code to see: > > > > 1) that we have the same fix as FreeBSD > > 2) that these values are in sync. > > > > > > Tobias > > > > Index: msdosfs_fat.c > > =================================================================== > > RCS file: /cvs/src/sys/msdosfs/msdosfs_fat.c,v > > retrieving revision 1.24 > > diff -u -p -r1.24 msdosfs_fat.c > > --- msdosfs_fat.c 11 Jun 2013 16:42:16 -0000 1.24 > > +++ msdosfs_fat.c 17 Jun 2014 21:32:55 -0000 > > @@ -866,7 +866,7 @@ fillinusemap(struct msdosfsmount *pmp) > > * Mark all clusters in use, we mark the free ones in the fat scan > > * loop further down. > > */ > > - for (cn = 0; cn < (pmp->pm_maxcluster + N_INUSEBITS) / N_INUSEBITS; cn++) > > + for (cn = 0; cn < howmany(pmp->pm_maxcluster + 1, N_INUSEBITS); cn++) > > pmp->pm_inusemap[cn] = (u_int)-1; > > > > /* > > Index: msdosfs_vfsops.c > > =================================================================== > > RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v > > retrieving revision 1.65 > > diff -u -p -r1.65 msdosfs_vfsops.c > > --- msdosfs_vfsops.c 27 May 2014 21:52:19 -0000 1.65 > > +++ msdosfs_vfsops.c 17 Jun 2014 21:32:55 -0000 > > @@ -517,7 +517,7 @@ msdosfs_mountfs(struct vnode *devvp, str > > * Allocate memory for the bitmap of allocated clusters, and then > > * fill it in. > > */ > > - bmapsiz = (pmp->pm_maxcluster + N_INUSEBITS - 1) / N_INUSEBITS; > > + bmapsiz = howmany(pmp->pm_maxcluster + 1, N_INUSEBITS); > > if (bmapsiz == 0 || SIZE_MAX / bmapsiz < sizeof(*pmp->pm_inusemap)) { > > /* detect multiplicative integer overflow */ > > error = EINVAL; > > > > Sure. Looks good to me. > > If you don't mind my asking, why the sudden flurry of msdosfs diffs, > and why did I get the short straw of looking at them all? > > .... Ken >
For what it is worth, I know Ken also likes looking at NFS diffs :)
