On review by Jan Tomko, for my earlier Patchv2, which had the return NULL replace by the return conn ,
here is the review:
```
>+cleanup:
> return conn;
>
> error:
> virDispatchError(NULL);
>- virObjectUnref(conn);
'conn' is still not marked as g_auto, so this unref needs to stay.
> conn = NULL;
> goto cleanup;
These two lines can be replaced by return NULL, and the cleanup: label
above removed.
```
here is the review:
```
>+cleanup:
> return conn;
>
> error:
> virDispatchError(NULL);
>- virObjectUnref(conn);
'conn' is still not marked as g_auto, so this unref needs to stay.
> conn = NULL;
> goto cleanup;
These two lines can be replaced by return NULL, and the cleanup: label
above removed.
```
I went ahead with what this review meant, and initially my PATCHv1 also did the same thing return NULL, because if there is an error the function is supposed to return NULL anyways.
On Mon, Mar 2, 2020 at 2:08 PM Peter Krempa <pkrempa@xxxxxxxxxx> wrote:
On Mon, Mar 02, 2020 at 07:22:03 +0530, Gaurav Agrawal wrote:
> Signed-off-by: Gaurav Agrawal <agrawalgaurav@xxxxxxxxx>
> ---
> src/admin/libvirt-admin.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
[...]
> @@ -251,16 +251,11 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> if (remoteAdminConnectOpen(conn, flags) < 0)
> goto error;
>
> - cleanup:
> - VIR_FREE(sock_path);
> - VIR_FREE(uristr);
> - return conn;
Removing this 'return' will make this function ALWAYS return NULL when
the control flow now continues through the error label.
>
> error:
> virDispatchError(NULL);
> virObjectUnref(conn);
> - conn = NULL;
> - goto cleanup;
> + return NULL;
> }
