On 17 June 2014 17:43, Tobias Stoeckmann <tob...@stoeckmann.org> 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
