On Sat, 23 May 2009 20:12:05 +0200, Dai Ngo <Dai.Ngo at sun.com> wrote:
> I'd like to have code review for a minor change to fix 6676419. > > The panic was caused by dereferencing a NULL pointer when the > server returned an error for the getattr op in the compound request. > > The fix is to add a check, proposed in the "Suggested Fix" section > of the CR, of the status of the op before accessing its reply result. > > The webrev is here: > http://cr.opensolaris.org/~dain/6676419/ looks sane to me, but I do wonder why a simple, preceding if (res.status == NFS4_OK) { 3114 /* getattr lease_time res */ 3115 if (res.array_len >= 2) { 3117 garp = &res.array[1].nfs_resop4_u.opgetattr.ga_res; .... is not being used instead. for the sake of nfs4setclientid_otw() it does not matter to know if the OP_PUTROOTFH _or_ the OP_GETATTR failed in detail, in either case we do not want to look at possibly existing GETATTR results. ie. if PUTROOTFH already failed, there's shouldn't be anything to expect from the following GETATTR at least for my understanding. I'd favor fixing it this way. --- frankB
