On 03/02/10 16:20 -0600, Tom Haynes wrote: > Jan Kryl wrote: > > Hi Rob, > > > > On 03/02/10 11:53 -0700, Robert Thurlow wrote: > > > >> Jan Kryl wrote: > >> > >>> Hi, > >>> please could you review the following fix for 6920999? > >>> webrev: > >>> http://cr.opensolaris.org/~jkryl/nfs-utf8/ > >>> > >> Hi Jan, > >> > >> The changes look good to me except for something related > >> to the ASSERT change Tom mentioned. I think ASSERTs and > >> a runtime check should be put in nfscmd_findmap(), which > >> is the highest point that is vulnerable to a stray NULL > >> pointer. > >> > >> > > I have removed asserts and added a NULL-test in > > nfscmd_findmap() as you have suggested. I think that > > the asserts aren't usefull anymore now when we test > > NULL values for both debug and non-debug kernels. > > > > > > The asserts are still useful in that they let us know about > a case that should never happen. In a debug kernel, we > want to know about this condition. I.e., it probably > will be hit by a developer during unit testing when > they introduce a bug in this area. > > In a non-debug kernel, for this case, we happen to be > able to recover in a fashion that can be handled by > the caller. In most cases like this, a panic occurs > and we get a customer support call. > > So please add the asserts back in - we'd like some > observability to the issue. > Sounds reasonable. Webrev updated:
http://cr.opensolaris.org/~jkryl/nfs-utf8-3/ thanks Jan
