[libvirt] Re: libvirt code review

[Daniel P. Berrange]

On Tue, Nov 10, 2009 at 03:40:59PM +0100, Daniel Veillard wrote:
> commit e74427b0570f4389b77c035dc093312f90b3d1e1
> Author: Daniel Veillard <veillard at redhat.com>
> Date:   Tue Nov 10 14:40:22 2009 +0100
> 
>     Various fixes following a code review part 2
>     
>     * daemon/libvirtd.c tools/virsh.c: Steve Grubb <sgrubb at redhat.com> found
>       a few more issues
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index daf06bc..2fcd9a9 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -359,7 +359,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
>                           void *opaque) {
>      struct qemud_server *server = (struct qemud_server *)opaque;
>      siginfo_t siginfo;
> -    int ret;
>  
>      virMutexLock(&server->lock);
>  
> @@ -371,8 +370,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
>          return;
>      }
>  
> -    ret = 0;
> -
>      switch (siginfo.si_signo) {
>      case SIGHUP:
>          VIR_INFO0(_("Reloading configuration on SIGHUP"));
> @@ -392,9 +389,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
>          break;
>      }
>  
> -    if (ret != 0)
> -        server->quitEventThread = 1;
> -
>      virMutexUnlock(&server->lock);
>  }
>  
> diff --git a/tools/virsh.c b/tools/virsh.c
> index f8e6ce4..0d0ebca 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -4627,8 +4627,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
>  
>      if (format) {
>          virBufferAddLit(&buf, "  <target>\n");
> -        if (format)
> -            virBufferVSprintf(&buf, "    <format type='%s'/>\n",format);
> +        virBufferVSprintf(&buf, "    <format type='%s'/>\n",format);
>          virBufferAddLit(&buf, "  </target>\n");
>      }
>      virBufferAddLit(&buf, "</volume>\n");
> @@ -6835,6 +6834,7 @@ editWriteToTempFile (vshControl *ctl, const char *doc)
>      if (fd == -1) {
>          vshError(ctl, _("mkstemp: failed to create temporary file: %s"),
>                   strerror(errno));
> +        free (ret);
>          return NULL;
>      }
>  
> @@ -7675,7 +7675,7 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd,
>          *name = n;
>  
>      /* try it by UUID */
> -    if (network==NULL && (flag & VSH_BYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) {
> +    if ((flag & VSH_BYUUID) && (strlen(n) == VIR_UUID_STRING_BUFLEN-1)) {
>          vshDebug(ctl, 5, "%s: <%s> trying as network UUID\n",
>                   cmd->def->name, optname);
>          network = virNetworkLookupByUUIDString(ctl->conn, n);
> @@ -7715,7 +7715,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd,
>          *name = n;
>  
>      /* try it by NAME */
> -    if ((iface == NULL) && (flag & VSH_BYNAME)) {
> +    if ((flag & VSH_BYNAME)) {
>          vshDebug(ctl, 5, "%s: <%s> trying as interface NAME\n",
>                   cmd->def->name, optname);
>          iface = virInterfaceLookupByName(ctl->conn, n);
> @@ -7752,13 +7752,13 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname,
>          *name = n;
>  
>      /* try it by UUID */
> -    if (pool==NULL && (flag & VSH_BYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) {
> +    if ((flag & VSH_BYUUID) && (strlen(n) == VIR_UUID_STRING_BUFLEN-1)) {
>          vshDebug(ctl, 5, "%s: <%s> trying as pool UUID\n",
>                   cmd->def->name, optname);
>          pool = virStoragePoolLookupByUUIDString(ctl->conn, n);
>      }
>      /* try it by NAME */
> -    if (pool==NULL && (flag & VSH_BYNAME)) {
> +    if (pool == NULL && (flag & VSH_BYNAME)) {
>          vshDebug(ctl, 5, "%s: <%s> trying as pool NAME\n",
>                   cmd->def->name, optname);
>          pool = virStoragePoolLookupByName(ctl->conn, n);

ACK

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|