[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/9] phyp: Fix memory/session leaks and potential invalid frees



On Fri, Nov 06, 2009 at 04:28:00AM +0100, Matthias Bolte wrote:
> ---
>  src/phyp/phyp_driver.c |  285 ++++++++++++++++++++++++++++-------------------
>  src/phyp/phyp_driver.h |    2 +
>  2 files changed, 172 insertions(+), 115 deletions(-)
> 
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index 8e199ee..5379cd3 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
> @@ -71,7 +71,7 @@ phypOpen(virConnectPtr conn,
>  {
>      LIBSSH2_SESSION *session = NULL;
>      ConnectionData *connection_data = NULL;
> -    char *string;
> +    char *string = NULL;
>      size_t len = 0;
>      int internal_socket;
>      uuid_tablePtr uuid_table = NULL;
> @@ -128,19 +128,17 @@ phypOpen(virConnectPtr conn,
>          goto failure;
>      }
>  
> -    if (VIR_ALLOC_N(managed_system, len) < 0) {
> +    /* need to shift one byte in order to remove the first "/" of URI component */
> +    if (conn->uri->path[0] == '/')
> +        managed_system = strdup(conn->uri->path + 1);
> +    else
> +        managed_system = strdup(conn->uri->path);
> +
> +    if (!managed_system) {
>          virReportOOMError(conn);
>          goto failure;
>      }
>  
> -    managed_system = strdup(conn->uri->path);
> -    if (!managed_system)
> -        goto failure;
> -
> -    /* need to shift one byte in order to remove the first "/" of URI component */
> -    if (managed_system[0] == '/')
> -        managed_system++;
> -
>      /* here we are handling only the first component of the path,
>       * so skipping the second:
>       * */
> @@ -187,11 +185,19 @@ phypOpen(virConnectPtr conn,
>      return VIR_DRV_OPEN_SUCCESS;
>  
>    failure:
> -    virCapabilitiesFree(phyp_driver->caps);
> -    VIR_FREE(phyp_driver->managed_system);
> -    VIR_FREE(phyp_driver);
> -    VIR_FREE(uuid_table);
> -    VIR_FREE(uuid_table->lpars);
> +    if (phyp_driver != NULL) {
> +        virCapabilitiesFree(phyp_driver->caps);
> +        VIR_FREE(phyp_driver->managed_system);
> +        VIR_FREE(phyp_driver);
> +    }
> +
> +    phypUUIDTable_Free(uuid_table);
> +
> +    if (session != NULL) {
> +        libssh2_session_disconnect(session, "Disconnecting...");
> +        libssh2_session_free(session);
> +    }
> +
>      VIR_FREE(connection_data);
>      VIR_FREE(string);
>  
> @@ -209,8 +215,7 @@ phypClose(virConnectPtr conn)
>      libssh2_session_free(session);
>  
>      virCapabilitiesFree(phyp_driver->caps);
> -    VIR_FREE(phyp_driver->uuid_table);
> -    VIR_FREE(phyp_driver->uuid_table->lpars);
> +    phypUUIDTable_Free(phyp_driver->uuid_table);
>      VIR_FREE(phyp_driver->managed_system);
>      VIR_FREE(phyp_driver);
>      VIR_FREE(connection_data);
> @@ -281,7 +286,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>          virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                        VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                        _("Failure establishing SSH session."));
> -        goto err;
> +        goto disconnect;
>      }
>  
>      /* Trying authentication by pubkey */
> @@ -305,7 +310,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>              virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                            VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                            _("No authentication callback provided."));
> -            goto err;
> +            goto disconnect;
>          }
>  
>          for (i = 0; i < auth->ncredtype; i++) {
> @@ -317,7 +322,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>              virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                            VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                            _("Required credentials are not supported."));
> -            goto err;
> +            goto disconnect;
>          }
>  
>          int res =
> @@ -327,7 +332,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>              virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                            VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                            _("Unable to fetch credentials."));
> -            goto err;
> +            goto disconnect;
>          }
>  
>          if (creds[0].result) {
> @@ -336,9 +341,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>              virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                            VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                            _("Unable to get password certificates"));
> -            libssh2_session_disconnect(session, "Disconnecting...");
> -            libssh2_session_free(session);
> -            goto err;
> +            goto disconnect;
>          }
>  
>          while ((rc =
> @@ -350,16 +353,19 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
>              virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP,
>                            VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s",
>                            _("Authentication failed"));
> -            libssh2_session_disconnect(session, "Disconnecting");
> -            libssh2_session_free(session);
> -            goto err;
> +            goto disconnect;
>          } else
>              goto exit;
>      }
> +  disconnect:
> +    libssh2_session_disconnect(session, "Disconnecting...");
> +    libssh2_session_free(session);
>    err:
> +    VIR_FREE(password);
>      return NULL;
>  
>    exit:
> +    VIR_FREE(password);
>      return session;
>  }
>  
> @@ -456,7 +462,8 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system,
>      int exit_status = 0;
>      int lpar_id = 0;
>      char *char_ptr;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>  
>      if (virAsprintf(&cmd,
>                      "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id",
> @@ -465,7 +472,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system,
>          goto err;
>      }
>  
> -    const char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (exit_status < 0 || ret == NULL)
>          goto err;
> @@ -474,10 +481,12 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system,
>          goto err;
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return lpar_id;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return -1;
>  }
>  
> @@ -486,7 +495,8 @@ static char *
>  phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system,
>                  unsigned int lpar_id, virConnectPtr conn)
>  {
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>  
>      if (virAsprintf(&cmd,
> @@ -496,7 +506,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system,
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (ret == NULL)
>          goto err;
> @@ -514,6 +524,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system,
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return NULL;
>  }
>  
> @@ -553,7 +564,8 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id,
>  {
>      ConnectionData *connection_data = conn->networkPrivateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      char *char_ptr;
>      int memory = 0;
>      int exit_status = 0;
> @@ -579,7 +591,7 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id,
>          }
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (ret == NULL)
>          goto err;
> @@ -596,10 +608,12 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id,
>          goto err;
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return memory;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return 0;
>  
>  }
> @@ -625,7 +639,8 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system,
>  {
>      ConnectionData *connection_data = conn->networkPrivateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>      int vcpus = 0;
>  
> @@ -646,7 +661,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system,
>              goto err;
>          }
>      }
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (ret == NULL)
>          goto err;
> @@ -663,10 +678,12 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system,
>          goto err;
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return (unsigned long) vcpus;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return 0;
>  }
>  
> @@ -676,7 +693,8 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system,
>  {
>      ConnectionData *connection_data = conn->networkPrivateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      char *char_ptr;
>      int remote_slot = 0;
>      int exit_status = 0;
> @@ -688,7 +706,7 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system,
>          virReportOOMError(conn);
>          goto err;
>      }
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (ret == NULL)
>          goto err;
> @@ -705,10 +723,12 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system,
>          goto err;
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return remote_slot;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return -1;
>  }
>  
> @@ -718,9 +738,12 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system,
>  {
>      ConnectionData *connection_data = conn->networkPrivateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int remote_slot = 0;
>      int exit_status = 0;
> +    char *char_ptr;
> +    char *backing_device = NULL;
>  
>      if ((remote_slot =
>           phypGetRemoteSlot(conn, managed_system, lpar_name)) == -1)
> @@ -734,9 +757,9 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system,
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
> -    if (ret == NULL)
> +    if (exit_status < 0 || ret == NULL)
>          goto err;
>  
>      /* here is a little trick to deal returns of this kind:
> @@ -746,31 +769,38 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system,
>       * the information we really need is only lv01, so we
>       * need to skip a lot of things on the string.
>       * */
> -    char *backing_device = strchr(ret, '/');
> +    char_ptr = strchr(ret, '/');
>  
> -    if (backing_device) {
> -        backing_device++;
> -        if (backing_device[0] == '/')
> -            backing_device++;
> +    if (char_ptr) {
> +        char_ptr++;
> +        if (char_ptr[0] == '/')
> +            char_ptr++;
>          else
>              goto err;
> +
> +        backing_device = strdup(char_ptr);
> +
> +        if (backing_device == NULL) {
> +            virReportOOMError(conn);
> +            goto err;
> +        }
>      } else {
>          backing_device = ret;
> +        ret = NULL;
>      }
>  
> -    char *char_ptr = strchr(backing_device, '\n');
> +    char_ptr = strchr(backing_device, '\n');
>  
>      if (char_ptr)
>          *char_ptr = '\0';
>  
> -    if (exit_status < 0 || backing_device == NULL)
> -        goto err;
> -
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return backing_device;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return NULL;
>  
>  }
> @@ -781,44 +811,41 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id)
>      ConnectionData *connection_data = conn->networkPrivateData;
>      phyp_driverPtr phyp_driver = conn->privateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>      char *char_ptr = NULL;
>      char *managed_system = phyp_driver->managed_system;
> +    int state = VIR_DOMAIN_NOSTATE;
>  
>      if (virAsprintf(&cmd,
>                      "lssyscfg -r lpar -m %s -F state --filter lpar_ids=%d",
>                      managed_system, lpar_id) < 0) {
>          virReportOOMError(conn);
> -        goto err;
> +        goto cleanup;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
> -    if (ret == NULL)
> -        goto err;
> +    if (exit_status < 0 || ret == NULL)
> +        goto cleanup;
>  
>      char_ptr = strchr(ret, '\n');
>  
>      if (char_ptr)
>          *char_ptr = '\0';
>  
> -    if (exit_status < 0 || ret == NULL)
> -        goto err;
> -
> -    VIR_FREE(cmd);
>      if (STREQ(ret, "Running"))
> -        return VIR_DOMAIN_RUNNING;
> +        state = VIR_DOMAIN_RUNNING;
>      else if (STREQ(ret, "Not Activated"))
> -        return VIR_DOMAIN_SHUTOFF;
> +        state = VIR_DOMAIN_SHUTOFF;
>      else if (STREQ(ret, "Shutting Down"))
> -        return VIR_DOMAIN_SHUTDOWN;
> -    else
> -        goto err;
> +        state = VIR_DOMAIN_SHUTDOWN;
>  
> -  err:
> +  cleanup:
>      VIR_FREE(cmd);
> -    return VIR_DOMAIN_NOSTATE;
> +    VIR_FREE(ret);
> +    return state;
>  }
>  
>  int
> @@ -827,7 +854,8 @@ phypGetVIOSPartitionID(virConnectPtr conn)
>      ConnectionData *connection_data = conn->networkPrivateData;
>      phyp_driverPtr phyp_driver = conn->privateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>      int id = -1;
>      char *char_ptr;
> @@ -840,7 +868,7 @@ phypGetVIOSPartitionID(virConnectPtr conn)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (exit_status < 0 || ret == NULL)
>          goto err;
> @@ -848,10 +876,13 @@ phypGetVIOSPartitionID(virConnectPtr conn)
>      if (virStrToLong_i(ret, &char_ptr, 10, &id) == -1)
>          goto err;
>  
> +    VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return id;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return -1;
>  }
>  
> @@ -861,44 +892,41 @@ phypDiskType(virConnectPtr conn, char *backing_device)
>      phyp_driverPtr phyp_driver = conn->privateData;
>      ConnectionData *connection_data = conn->networkPrivateData;
>      LIBSSH2_SESSION *session = connection_data->session;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>      char *char_ptr;
>      char *managed_system = phyp_driver->managed_system;
>      int vios_id = phyp_driver->vios_id;
> +    int disk_type = -1;
>  
>      if (virAsprintf(&cmd,
>                      "viosvrcmd -m %s -p %d -c \"lssp -field name type "
>                      "-fmt , -all|grep %s|sed -e 's/^.*,//g'\"",
>                      managed_system, vios_id, backing_device) < 0) {
>          virReportOOMError(conn);
> -        goto err;
> +        goto cleanup;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
> -    if (ret == NULL)
> -        goto err;
> +    if (exit_status < 0 || ret == NULL)
> +        goto cleanup;
>  
>      char_ptr = strchr(ret, '\n');
>  
>      if (char_ptr)
>          *char_ptr = '\0';
>  
> -    if (exit_status < 0 || ret == NULL)
> -        goto err;
> -
> -    VIR_FREE(cmd);
>      if (STREQ(ret, "LVPOOL"))
> -        return VIR_DOMAIN_DISK_TYPE_BLOCK;
> +        disk_type = VIR_DOMAIN_DISK_TYPE_BLOCK;
>      else if (STREQ(ret, "FBPOOL"))
> -        return VIR_DOMAIN_DISK_TYPE_FILE;
> -    else
> -        goto err;
> +        disk_type = VIR_DOMAIN_DISK_TYPE_FILE;
>  
> -  err:
> +  cleanup:
>      VIR_FREE(cmd);
> -    return -1;
> +    VIR_FREE(ret);
> +    return disk_type;
>  }
>  
>  /* This is a generic function that won't be used directly by
> @@ -918,7 +946,8 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
>      int exit_status = 0;
>      int ndom = 0;
>      char *char_ptr;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      char *managed_system = phyp_driver->managed_system;
>      const char *state;
>  
> @@ -936,7 +965,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (exit_status < 0 || ret == NULL)
>          goto err;
> @@ -945,10 +974,12 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
>          goto err;
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return ndom;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return 0;
>  }
>  
> @@ -984,7 +1015,8 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
>      char *char_ptr;
>      unsigned int i = 0, j = 0;
>      char id_c[10];
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      const char *state;
>  
>      if (type == 0)
> @@ -1001,7 +1033,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
>          virReportOOMError(conn);
>          goto err;
>      }
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      /* I need to parse the textual return in order to get the ret */
>      if (exit_status < 0 || ret == NULL)
> @@ -1023,10 +1055,12 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
>      }
>  
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return got;
>  
>    err:
>      VIR_FREE(cmd);
> +    VIR_FREE(ret);
>      return 0;
>  }
>  
> @@ -1045,8 +1079,11 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames)
>      char *managed_system = phyp_driver->managed_system;
>      int exit_status = 0;
>      int got = 0;
> -    char *cmd;
> -    char *domains;
> +    int i;
> +    char *cmd = NULL;
> +    char *ret = NULL;
> +    char *domains = NULL;
> +    char *char_ptr2 = NULL;
>  
>      if (virAsprintf
>          (&cmd,
> @@ -1056,42 +1093,37 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> -
> -    if (VIR_ALLOC(domains) < 0)
> -        virReportOOMError(conn);
> -
> -    domains = strdup(ret);
> -    if (!domains)
> -        goto err;
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
> -    char *char_ptr2 = NULL;
>      /* I need to parse the textual return in order to get the domains */
> -    if (exit_status < 0 || domains == NULL)
> +    if (exit_status < 0 || ret == NULL)
>          goto err;
>      else {
> +        domains = ret;
> +
>          while (got < nnames) {
>              char_ptr2 = strchr(domains, '\n');
>  
>              if (char_ptr2) {
>                  *char_ptr2 = '\0';
> -                if (!strdup(domains))
> +                if ((names[got++] = strdup(domains)) == NULL) {
> +                    virReportOOMError(conn);
>                      goto err;
> -                names[got] = strdup(domains);
> +                }
>                  char_ptr2++;
>                  domains = char_ptr2;
> -                got++;
>              }
>          }
>      }
>  
> -    VIR_FREE(domains);
>      VIR_FREE(cmd);
>      VIR_FREE(ret);
>      return got;
>  
>    err:
> -    VIR_FREE(domains);
> +    for (i = 0; i < got; i++)
> +        VIR_FREE(names[i]);
> +    VIR_FREE(cmd);
>      VIR_FREE(ret);
>      return 0;
>  }
> @@ -1242,7 +1274,8 @@ phypDomainResume(virDomainPtr dom)
>      LIBSSH2_SESSION *session = connection_data->session;
>      char *managed_system = phyp_driver->managed_system;
>      int exit_status = 0;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>  
>      if (virAsprintf
>          (&cmd,
> @@ -1252,7 +1285,7 @@ phypDomainResume(virDomainPtr dom)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, dom->conn);
> +    ret = phypExec(session, cmd, &exit_status, dom->conn);
>  
>      if (exit_status < 0)
>          goto err;
> @@ -1275,7 +1308,8 @@ phypDomainShutdown(virDomainPtr dom)
>      LIBSSH2_SESSION *session = connection_data->session;
>      char *managed_system = phyp_driver->managed_system;
>      int exit_status = 0;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>  
>      if (virAsprintf
>          (&cmd,
> @@ -1285,7 +1319,7 @@ phypDomainShutdown(virDomainPtr dom)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, dom->conn);
> +    ret = phypExec(session, cmd, &exit_status, dom->conn);
>  
>      if (exit_status < 0)
>          goto err;
> @@ -1331,7 +1365,8 @@ phypDomainDestroy(virDomainPtr dom)
>      LIBSSH2_SESSION *session = connection_data->session;
>      char *managed_system = phyp_driver->managed_system;
>      int exit_status = 0;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>  
>      if (virAsprintf
>          (&cmd,
> @@ -1340,7 +1375,7 @@ phypDomainDestroy(virDomainPtr dom)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, dom->conn);
> +    ret = phypExec(session, cmd, &exit_status, dom->conn);
>  
>      if (exit_status < 0)
>          goto err;
> @@ -1406,7 +1441,8 @@ phypDomainCreateAndStart(virConnectPtr conn,
>  
>    err:
>      virDomainDefFree(def);
> -    VIR_FREE(dom);
> +    if (dom)
> +        virUnrefDomain(dom);
>      return NULL;
>  }
>  
> @@ -1474,7 +1510,8 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus)
>      LIBSSH2_SESSION *session = connection_data->session;
>      char *managed_system = phyp_driver->managed_system;
>      int exit_status = 0;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      char operation;
>      unsigned long ncpus = 0;
>      unsigned int amount = 0;
> @@ -1507,7 +1544,7 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, dom->conn);
> +    ret = phypExec(session, cmd, &exit_status, dom->conn);
>  
>      if (exit_status < 0) {
>          VIR_ERROR("%s",
> @@ -1606,7 +1643,8 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
>      phyp_driverPtr phyp_driver = conn->privateData;
>      LIBSSH2_SESSION *session = connection_data->session;
>      char *managed_system = phyp_driver->managed_system;
> -    char *cmd;
> +    char *cmd = NULL;
> +    char *ret = NULL;
>      int exit_status = 0;
>  
>      if (virAsprintf
> @@ -1620,7 +1658,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
>          goto err;
>      }
>  
> -    char *ret = phypExec(session, cmd, &exit_status, conn);
> +    ret = phypExec(session, cmd, &exit_status, conn);
>  
>      if (exit_status < 0) {
>          VIR_ERROR("%s\"%s\"", "Unable to create LPAR. Reason: ", ret);
> @@ -1728,8 +1766,10 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
>  
>              rc = read(fd, buffer, sizeof(int));
>              if (rc == sizeof(int)) {
> -                if (VIR_ALLOC(uuid_table->lpars[i]) < 0)
> +                if (VIR_ALLOC(uuid_table->lpars[i]) < 0) {
>                      virReportOOMError(conn);
> +                    goto err;
> +                }
>                  uuid_table->lpars[i]->id = (*buffer);
>              } else {
>                  VIR_WARN("%s",
> @@ -1851,6 +1891,21 @@ phypUUIDTable_Init(virConnectPtr conn)
>      return -1;
>  }
>  
> +void
> +phypUUIDTable_Free(uuid_tablePtr uuid_table)
> +{
> +    int i;
> +
> +    if (uuid_table == NULL)
> +        return;
> +
> +    for (i = 0; i < uuid_table->nlpars; i++)
> +        VIR_FREE(uuid_table->lpars[i]);
> +
> +    VIR_FREE(uuid_table->lpars);
> +    VIR_FREE(uuid_table);
> +}
> +
>  int
>  phypUUIDTable_Push(virConnectPtr conn)
>  {
> diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h
> index 5ec12be..d05f184 100644
> --- a/src/phyp/phyp_driver.h
> +++ b/src/phyp/phyp_driver.h
> @@ -92,6 +92,8 @@ int phypUUIDTable_Push(virConnectPtr conn);
>  
>  int phypUUIDTable_Init(virConnectPtr conn);
>  
> +void phypUUIDTable_Free(uuid_tablePtr uuid_table);
> +
>  int escape_specialcharacters(char *src, char *dst, size_t dstlen);
>  
>  int waitsocket(int socket_fd, LIBSSH2_SESSION * session);

  A lot of cleanups indeed, thanks a lot for going through the code,

   ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]