Hi Nithin,
Thanks a lot for making the changes.
One minor comment:
if ((NlBufCopyAtTail(nlBuf, NULL, len)) == FALSE) {
return NULL; <-- we can do ret=NULL so that we have only one return
statement in this function.
}
Regarding naming:
I have no strong preference. I kept CopyAt in the function name to be
consistent with other functions. But if leads to confusion then we can move to
the naming which sam has recommended. But yes, it need not come in this series.
We can take care of it in a separate patch.
Acked-by: Ankur Sharma <ankursha...@vmware.com>
Thanks.
Regards,
Ankur
________________________________________
From: dev <dev-boun...@openvswitch.org> on behalf of Nithin Raju
<nit...@vmware.com>
Sent: Monday, September 15, 2014 12:26 AM
To: Samuel Ghinet
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH 1/2] datapath-windows: fix bug in
NlBufCopyAtTailUninit
On Sep 14, 2014, at 8:18 PM, Samuel Ghinet <sghi...@cloudbasesolutions.com>
wrote:
> At first on reading this code, I was wondering "why?"
> Now I think I understand: the function zeroes memory, and must return the ptr
> to 'next' item to be filled (which is called "tail"). Am I correct?
Yes, that is the bug. The pointer returned is the same pointer at where memory
was zeroed.
> Also, I think it would be nice if you could rename NlBufCopyAtTailUninit ->
> NlBufZeroAtTailUninit or smth similar, because it does not actually do a
> copy, and the name makes it a bit confusing.
> But perhaps this would also work as a separate patch.
Sure, I'll talk to Ankur about this since he designed and wrote this code. He
might have some thoughts too.
Thanks for the review. You can ack the v2 patch, which I'll send out.
Thanks,
-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=8l2QScg0CJXimjkl0edvkTdBNBKzmqyDjnxQXpU1lAI%3D%0A&s=51916c5716f8a68bc7a99ccf8df1733f28570489b3944675df508d87c5c82955
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
