diff mbox

[ovs-dev,v2] datapath-windows: Proper cleanup in case of driver init failure

Message ID 1441089130-8690-1-git-send-email-svinturis@cloudbasesolutions.com
State Accepted
Headers show

Commit Message

Sorin Vinturis Sept. 1, 2015, 2:21 p.m. UTC
Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
---
 datapath-windows/ovsext/Driver.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Ben Pfaff Sept. 1, 2015, 3:28 p.m. UTC | #1
Nithin, does this version resolve the concerns you had about v1?

Thanks,

Ben.

On Tue, Sep 01, 2015 at 02:21:09PM +0000, Sorin Vinturis wrote:
> Signed-off-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
> ---
>  datapath-windows/ovsext/Driver.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
> index 13fcde2..853886e 100644
> --- a/datapath-windows/ovsext/Driver.c
> +++ b/datapath-windows/ovsext/Driver.c
> @@ -139,18 +139,26 @@ DriverEntry(PDRIVER_OBJECT driverObject,
>      driverObject->DriverUnload = OvsExtUnload;
>  
>      status = NdisFRegisterFilterDriver(driverObject,
> -                                       (NDIS_HANDLE) gOvsExtDriverObject,
> -                                       &driverChars, &gOvsExtDriverHandle);
> +                                       (NDIS_HANDLE)gOvsExtDriverObject,
> +                                       &driverChars,
> +                                       &gOvsExtDriverHandle);
>      if (status != NDIS_STATUS_SUCCESS) {
> -        return status;
> +        goto cleanup;
>      }
>  
> -    /* Create the communication channel for usersapce. */
> +    /* Create the communication channel for userspace. */
>      status = OvsCreateDeviceObject(gOvsExtDriverHandle);
>      if (status != NDIS_STATUS_SUCCESS) {
> +        goto cleanup;
> +    }
> +
> +cleanup:
> +    if (status != NDIS_STATUS_SUCCESS){
>          OvsCleanup();
> -        NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> -        gOvsExtDriverHandle = NULL;
> +        if (gOvsExtDriverHandle) {
> +            NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> +            gOvsExtDriverHandle = NULL;
> +        }
>      }
>  
>      return status;
> -- 
> 1.9.0.msysgit.0
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
Nithin Raju Sept. 1, 2015, 3:36 p.m. UTC | #2
hi Ben,
Yes, I do understand the need for the patch now. Sorin pointed it out.

However, I just sent out a comment which if correct reduces the size of the patch. Upto Sorin to take up the comment or not.

I am ok with the current version of the patch too, since there’s no correctness issue.
Acked-by: Nithin Raju <nithin@vmware.com>


thanks,
-- Nithin

> On Sep 1, 2015, at 8:28 AM, Ben Pfaff <blp@nicira.com> wrote:

> 

> Nithin, does this version resolve the concerns you had about v1?

> 

> Thanks,
Ben Pfaff Sept. 1, 2015, 3:37 p.m. UTC | #3
Sorin, do you want me to take this version or will you post a v3?

Thanks,

Ben.

On Tue, Sep 01, 2015 at 03:36:08PM +0000, Nithin Raju wrote:
> hi Ben,
> Yes, I do understand the need for the patch now. Sorin pointed it out.
> 
> However, I just sent out a comment which if correct reduces the size of the patch. Upto Sorin to take up the comment or not.
> 
> I am ok with the current version of the patch too, since there’s no correctness issue.
> Acked-by: Nithin Raju <nithin@vmware.com>
> 
> thanks,
> -- Nithin
> 
> > On Sep 1, 2015, at 8:28 AM, Ben Pfaff <blp@nicira.com> wrote:
> > 
> > Nithin, does this version resolve the concerns you had about v1?
> > 
> > Thanks,
Sorin Vinturis Sept. 1, 2015, 5:17 p.m. UTC | #4
Ben, the v2 version is fine. Please commit this version to the 2.4 branch also.

Thanks,
Sorin

-----Original Message-----
From: Ben Pfaff [mailto:blp@nicira.com] 

Sent: Tuesday, 1 September, 2015 18:37
To: Sorin Vinturis
Cc: dev@openvswitch.org; Nithin Raju
Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Proper cleanup in case of driver init failure

Sorin, do you want me to take this version or will you post a v3?

Thanks,

Ben.

On Tue, Sep 01, 2015 at 03:36:08PM +0000, Nithin Raju wrote:
> hi Ben,

> Yes, I do understand the need for the patch now. Sorin pointed it out.

> 

> However, I just sent out a comment which if correct reduces the size of the patch. Upto Sorin to take up the comment or not.

> 

> I am ok with the current version of the patch too, since there’s no correctness issue.

> Acked-by: Nithin Raju <nithin@vmware.com>

> 

> thanks,

> -- Nithin

> 

> > On Sep 1, 2015, at 8:28 AM, Ben Pfaff <blp@nicira.com> wrote:

> > 

> > Nithin, does this version resolve the concerns you had about v1?

> > 

> > Thanks,
Ben Pfaff Sept. 1, 2015, 5:23 p.m. UTC | #5
Thanks Sorin, applied to both branches.

On Tue, Sep 01, 2015 at 05:17:44PM +0000, Sorin Vinturis wrote:
> Ben, the v2 version is fine. Please commit this version to the 2.4 branch also.
> 
> Thanks,
> Sorin
> 
> -----Original Message-----
> From: Ben Pfaff [mailto:blp@nicira.com] 
> Sent: Tuesday, 1 September, 2015 18:37
> To: Sorin Vinturis
> Cc: dev@openvswitch.org; Nithin Raju
> Subject: Re: [ovs-dev] [PATCH v2] datapath-windows: Proper cleanup in case of driver init failure
> 
> Sorin, do you want me to take this version or will you post a v3?
> 
> Thanks,
> 
> Ben.
> 
> On Tue, Sep 01, 2015 at 03:36:08PM +0000, Nithin Raju wrote:
> > hi Ben,
> > Yes, I do understand the need for the patch now. Sorin pointed it out.
> > 
> > However, I just sent out a comment which if correct reduces the size of the patch. Upto Sorin to take up the comment or not.
> > 
> > I am ok with the current version of the patch too, since there’s no correctness issue.
> > Acked-by: Nithin Raju <nithin@vmware.com>
> > 
> > thanks,
> > -- Nithin
> > 
> > > On Sep 1, 2015, at 8:28 AM, Ben Pfaff <blp@nicira.com> wrote:
> > > 
> > > Nithin, does this version resolve the concerns you had about v1?
> > > 
> > > Thanks,
diff mbox

Patch

diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
index 13fcde2..853886e 100644
--- a/datapath-windows/ovsext/Driver.c
+++ b/datapath-windows/ovsext/Driver.c
@@ -139,18 +139,26 @@  DriverEntry(PDRIVER_OBJECT driverObject,
     driverObject->DriverUnload = OvsExtUnload;
 
     status = NdisFRegisterFilterDriver(driverObject,
-                                       (NDIS_HANDLE) gOvsExtDriverObject,
-                                       &driverChars, &gOvsExtDriverHandle);
+                                       (NDIS_HANDLE)gOvsExtDriverObject,
+                                       &driverChars,
+                                       &gOvsExtDriverHandle);
     if (status != NDIS_STATUS_SUCCESS) {
-        return status;
+        goto cleanup;
     }
 
-    /* Create the communication channel for usersapce. */
+    /* Create the communication channel for userspace. */
     status = OvsCreateDeviceObject(gOvsExtDriverHandle);
     if (status != NDIS_STATUS_SUCCESS) {
+        goto cleanup;
+    }
+
+cleanup:
+    if (status != NDIS_STATUS_SUCCESS){
         OvsCleanup();
-        NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
-        gOvsExtDriverHandle = NULL;
+        if (gOvsExtDriverHandle) {
+            NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
+            gOvsExtDriverHandle = NULL;
+        }
     }
 
     return status;