"Verma, Vishal L" <[email protected]> writes: > On Mon, 2019-08-05 at 18:39 +0530, Vaibhav Jain wrote: >> Thanks for reviewing this patch Vishal. I have prepped a v2 adressing >> all your review comments with one exception below: > > No problem, thanks for fixing this. > >> >> "Verma, Vishal L" <[email protected]> writes: >> >> > On Thu, 2019-07-04 at 08:21 +0530, Vaibhav Jain wrote: >> > > >> > >> > > + if (addr != MAP_FAILED) >> > > + addr = (void *) ((uintptr_t)addr + shift); >> > >> > The (uintptr_t) cast should be ok to drop, for v66 we are removing >> > the >> > pointer arithmatic warning: >> > https://patchwork.kernel.org/patch/11062697/ >> > >> > In fact, since 'shift' is in bytes, isn't an unsigned int cast >> > actually *wrong*? >> Not sure if I understand your review comment correctly. With uintptr_t >> cast and 'shift' in bytes, addr will be assigned 'addr + shift' >> instead >> of 'addr + shift * sizeof (unsigned int)' >> >> So I think the arithmetic I am doing here is correct. >> > Yes you're right - I conflated a (uintptr_t) cast with an (unsigned int) > cast. The latter would actually be wrong, but I see now that uintptr_t > is correct. However, given the above patch where we drop the pointer > arithmetic warning, I think it should be OK to manipulate the void > pointer directly, and this makes the line cleaner and more concise. > > Thanks, > -Vishal
Sure agreed. I have sent out a v2 patch titled "[PATCH v2] ndctl, check: Ensure mmap of BTT sections work with 64K page-sizes" with the changes. Thanks, -- Vaibhav Jain <[email protected]> Linux Technology Center, IBM India Pvt. Ltd. _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
