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 _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
