diff mbox

use virGetLastErrorMessage instead of virGetLastError to check for NULL in qemu

Message ID 56EB2803.4020908@redhat.com
State New
Headers show

Commit Message

Cole Robinson March 17, 2016, 9:56 p.m. UTC 
Thanks for the patch. Another commit message tip: keep all lines under 80
characters long, and even try to keep the first line under 60 so it's more
readable for various mail clients. For this patch I'd use a commit message like

  qemu: Don't duplicate virGetLastErrorMessage

  These uses of virGetLastError message are just duplicating
  virGetLastErrorMessage.

(not perfect... I'm not great at commit messages myself)

More comments below

On 03/17/2016 03:01 PM, Jovanka Gulicoska wrote:
> ---

>  src/qemu/qemu_capabilities.c | 18 ++++++++----------

>  src/qemu/qemu_driver.c       | 12 +++++-------

>  2 files changed, 13 insertions(+), 17 deletions(-)

> 

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c

> index b223837..af35f89 100644

> --- a/src/qemu/qemu_capabilities.c

> +++ b/src/qemu/qemu_capabilities.c

> @@ -3089,10 +3089,9 @@ virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const char *cacheDir)

>  

>      if (virQEMUCapsLoadCache(qemuCaps, capsfile, &qemuctime, &selfctime,

>                               &selfvers) < 0) {

> -        virErrorPtr err = virGetLastError();

> +        const char *err = virGetLastErrorMessage();

>          VIR_WARN("Failed to load cached caps from '%s' for '%s': %s",

> -                 capsfile, qemuCaps->binary, err ? NULLSTR(err->message) :

> -                 _("unknown error"));

> +                 capsfile, qemuCaps->binary, err);

>          virResetLastError();

>          ret = 0;

>          virQEMUCapsReset(qemuCaps);


These usages can be simplified even more. For example on top of this patch:

         virQEMUCapsReset(qemuCaps);


So please use that pattern instead and resubmit.

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index af35f89..7f91730 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3089,9 +3089,8 @@  virQEMUCapsInitCached(virQEMUCapsPtr qemuCaps, const
char *cacheDir)

     if (virQEMUCapsLoadCache(qemuCaps, capsfile, &qemuctime, &selfctime,
                              &selfvers) < 0) {
-        const char *err = virGetLastErrorMessage();
         VIR_WARN("Failed to load cached caps from '%s' for '%s': %s",
-                 capsfile, qemuCaps->binary, err);
+                 capsfile, qemuCaps->binary, virGetLastErrorMessage());
         virResetLastError();
         ret = 0;