On Sep 14, 2014, at 8:36 PM, Samuel Ghinet <[email protected]>
 wrote:

> Hello Nithin,
> 
> Overall, it looks ok.
> 
> Very minor things, though:
> (OvsDpFillInfo)
> o) could you please rename nlWrite into something more boolean-like, like 
> "ok" or whatever you prefer?
> When I first read that piece of code, I was thinking nlWrite is a ptr 
> returned by NlMsgPutHead. It confused me for a few minutes.
> 
> o) the variables datapath and dpStats are only used in the second if branch. 
> I think it would look better and cleaner if you declared them in that if 
> branch.
> 
> o) Also, a very minor thing, I think it would be nice if we could make a 
> typedef in, e.g., OvsDpInterfaceExt.h:
> typedef struct ovs_dp_stats OVS_DP_STATS;
> it would make the code in the windows driver more consistent with all the 
> other parts.

Thanks for the comments. I'll address all of them in the v2 patch.

Thanks,
-- Nithin

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to