From patchwork Thu Mar 17 21:56:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 64010 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp719503lbc; Thu, 17 Mar 2016 14:59:12 -0700 (PDT) X-Received: by 10.140.228.68 with SMTP id y65mr18705465qhb.78.1458251952234; Thu, 17 Mar 2016 14:59:12 -0700 (PDT) Return-Path: Received: from mx6-phx2.redhat.com (mx6-phx2.redhat.com. [209.132.183.39]) by mx.google.com with ESMTPS id w13si9573420qha.54.2016.03.17.14.59.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Mar 2016 14:59:12 -0700 (PDT) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.39 as permitted sender) client-ip=209.132.183.39; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.39 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx6-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2HLuMtc061971; Thu, 17 Mar 2016 17:56:23 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u2HLuKiV029088 for ; Thu, 17 Mar 2016 17:56:20 -0400 Received: from [10.3.113.66] (ovpn-113-66.phx2.redhat.com [10.3.113.66]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2HLuKQE013739; Thu, 17 Mar 2016 17:56:20 -0400 To: Jovanka Gulicoska , libvir-list@redhat.com References: <1458241311-27590-1-git-send-email-jovanka.gulicoska@gmail.com> From: Cole Robinson Message-ID: <56EB2803.4020908@redhat.com> Date: Thu, 17 Mar 2016 17:56:19 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1458241311-27590-1-git-send-email-jovanka.gulicoska@gmail.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-loop: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH] use virGetLastErrorMessage instead of virGetLastError to check for NULL in qemu X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com 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 --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;