Marcel Telka wrote: > On Wed, Jul 01, 2009 at 09:47:00AM -0500, Piyush Shivam wrote: >>> 2. What will happen when VOP_PATHCONF() will return 0 in l, resp. in val or >>> in pc_val? >>> >> Not sure if I understand. 0 in 1? or 0 or 1? Can you please explain when > > "0 in l" > > It is not 1 (one). It is l (el, lowercase L). > OK. Makes sense now. >> val or pc_val will have 0 or 1 in them as per the semantics of >> VOP_PATHCONF for _PC_FILESIZEBITS? > > It shouldn't, but are we (you) sure it won't? > The minimum acceptable value for FILESIZEBITS is 32. Could we rely on that > fact?
I guess at some level we have to rely on the correctness of PSARCed interfaces. So, yes, I will rely on the VOP_PATHCONF not returning 0 or 1 in l (lower case L), pc_val, and val. > > If you think so, then ok. No more questions :-). > >>> 3. This code is dangerous and/or confusing (assignment in if without a >>> test): >>> >>> 234 if (error = VOP_PATHCONF(vp, _PC_FILESIZEBITS, >>> &pc_val, cr, >>> 235 NULL)) >>> 236 return (error); >>> >>> I know this is out of the scope of your fix, but I would like to see it >>> fixed. >> Is this a correctness issue? or aesthetics? Can you give a specific >> example where this is an obvious issue? > > It is not obvious whether you wanted to assign (=) or compare (==), because in > if you in most cases want to compare. OK. I see how the reader may get confused. Thanks for clarifying. I think it is worth fixing this "out of scope" changes. :) -Piyush > >> I personally do not prefer the construct above myself, it can be hard to >> read, but at the same time I am not inclined to changing code in NFS as >> above, this late in the game. > > Ok. It is up to you whether you'll change it or not. > > Thank you >
