On Wed, Jul 04, 2012 at 01:45:08AM +0900, Izumi Tsutsui wrote: > > getdisksize() is not complicated; it tries DIOCGPART and then > > DIOCGWEDGEINFO and that's about it. > > Yes, but it is not implementation issue but design issue. > > > I think it's wrong for either of those to be returning a size of 0. > > Yes, but which one? getdisksize() or DIOCGPART in each driver?
There's nothing getdisksize can do besides return what the driver says. If the driver is reporting nonsense, it's wrong. > > > I can change it to KASSERT if we can assume that partinfo taken > > > from DIOCGPART must be initialized properly on all disk(9) devices, > > > i.e. if we can say disklab->d_secsize==0 means driver's bug, > > > but I could not find any evidence. > > > > > > All vfs mountfs functions are called in case of "root on generic" > > > via mountroot in setroot() so I thought returning error was better > > > than panic in that case, even after md(4) was fixed. > > > But if you don't agree it, please revert it. > > > > If we know it can happen, crashing isn't very productive. > > > > However, can you move the test to inside getdisksize()? Make it print > > a message and return EINVAL (or something). That way things other than > > msdosfs are also protected. > > It depends how the design issue is resolved. > If DIOCGPART must not return secsize=0, > KASSERT in getdisksize() seems reasonable. > If uninitialized secsize=0 from DIOCGPART is acceptable, > getdisksize() or each caller should have sanity checks. Sure, all I'm saying is that since we know it can happen, printing a message and returning an error (and not just when DIAGNOSTIC) is probably better than crashing. Once we think the drivers are fixed, then it can become an assertion. -- David A. Holland dholl...@netbsd.org
