On Mon, Mar 14, 2011 at 07:52:11PM -0400, Trond Myklebust wrote: > On Mon, 2011-03-14 at 18:22 -0400, J. Bruce Fields wrote: > > On Fri, Mar 11, 2011 at 12:13:55PM +0800, Mi Jinlong wrote: > > > > > > > > > J. Bruce Fields: > > > > On Tue, Mar 08, 2011 at 10:32:26PM +0100, roel wrote: > > > >> @@ -1215,7 +1215,7 @@ nfsd4_decode_create_session(struct nfsd4_compoundargs *argp, > > > >> READ_BUF(4); > > > >> READ32(dummy); > > > >> READ_BUF(dummy * 4); > > > >> - for (i = 0; i < dummy; ++i) > > > >> + for (j = 0; j < dummy; ++j) > > > >> READ32(dummy); > > > > > > We must not use dummy for index here. > > > After the first index, READ32(dummy) will change dummy!!!! > > > > Actually, wait, this is kind of silly. I don't see why we couldn't just > > skip the loop and do > > > > p += dummy; > > This is exactly why I _hate_ the READ*() macros and their ilk, and am > really happy we got rid of them in the client. Agreed, I'm all for getting rid of them completely. > READ_BUF() _sets_ p to whatever the value of argp->p is, and then > updates argp->p. It is just very very very hard to see that due to the > lack of transparency. > > IOW: You don't need the "p += dummy" either. That happens automatically > when you next invoke READ_BUF(). Yes, you're right, we could remove that silly loop entirely. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
