> On Jul 1, 2015, at 10:28 AM, Sorin Vinturis > <[email protected]> wrote: > > OvsPrepareFlow() returns an error only when the new flow allocation > fails. In this case HandleFlowPut() should return error without trying > to free the flow, thus avoiding the BSOD. > > Signed-off-by: Sorin Vinturis <[email protected]> > Reported-by: Sorin Vinturis <[email protected]> > Reported-at: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_91&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=1fAwiEJ0trEqEbnXCwc2WN_OdJ9ANd0-fuucgcbW9Bs&s=v7pTsmCFx-lu-RBhxiVv5cbM5jUrnSC6P6sP6dp-PRw&e= > > --- > This patch should be applied both on master and branch 2.4. > --- > datapath-windows/ovsext/Flow.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c > index 6fa10a3..b93f475 100644 > --- a/datapath-windows/ovsext/Flow.c > +++ b/datapath-windows/ovsext/Flow.c > @@ -2163,7 +2163,6 @@ HandleFlowPut(OvsFlowPut *put, > > status = OvsPrepareFlow(&KernelFlow, put, hash); > if (status != STATUS_SUCCESS) { > - FreeFlow(KernelFlow); > return STATUS_UNSUCCESSFUL; > }
I’m looking at OvsPrepareFlow(), and the only case where we the function can fail is if there’s a memory allocation failure at which point ‘KernelFlow’ would end up being NULL, so, FreeFlow() will actually ASSERT(). Moreover, I think it would be wrong for the caller to free up an object that is allocated within OvsPrepareFlow(). I believe that if the function OvsPrepareFlow() fails, it should take responsibility for cleaning the objects that were allocated in the context of the function, and not expect that caller to do so. thanks, -- Nithin _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
