From nobody Mon Sep 16 19:56:16 2024 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1706637051288796.4341777975905; Tue, 30 Jan 2024 09:50:51 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id 35D0A1E88; Tue, 30 Jan 2024 12:50:50 -0500 (EST) Received: from lists.libvirt.org.85.43.8.in-addr.arpa (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 3E8461CB8; Tue, 30 Jan 2024 12:11:40 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 9CB031C07; Tue, 30 Jan 2024 12:10:18 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 05E561D28 for ; Tue, 30 Jan 2024 12:08:34 -0500 (EST) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-562-XGfT887eMh6O3m9LZfhNxA-1; Tue, 30 Jan 2024 12:08:32 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8AAC8185A786 for ; Tue, 30 Jan 2024 17:08:32 +0000 (UTC) Received: from speedmetal.redhat.com (unknown [10.45.242.16]) by smtp.corp.redhat.com (Postfix) with ESMTP id F319D2166B31 for ; Tue, 30 Jan 2024 17:08:31 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: XGfT887eMh6O3m9LZfhNxA-1 From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 20/31] util: virpcivpd: Remove return value from virPCIVPDResourceUpdateKeyword Date: Tue, 30 Jan 2024 18:07:58 +0100 Message-ID: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: W4MOL45ZZSV7FMOSOIECVAEXWUW7XAIZ X-Message-ID-Hash: W4MOL45ZZSV7FMOSOIECVAEXWUW7XAIZ X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1706637053189100001 The function always succeeded and after the removal of programing error checks doesn't even have a 'return false' case. Additionally one of the tests in 'virpcivpdtest' tested that this function never failed on wrong data. Embrace this logic and remove the return value and adjust logging to VIR_DEBUG level to avoid spamming logs. Signed-off-by: Peter Krempa --- src/util/virpcivpd.c | 31 +++++++++++-------------------- src/util/virpcivpd.h | 8 +++++--- tests/virpcivpdtest.c | 38 ++++++++++++++++++-------------------- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/src/util/virpcivpd.c b/src/util/virpcivpd.c index 3beb405252..0021a88f2d 100644 --- a/src/util/virpcivpd.c +++ b/src/util/virpcivpd.c @@ -307,54 +307,48 @@ virPCIVPDResourceCustomUpsertValue(GPtrArray *arr, ch= ar index, const char *const * used in XML elements. For vendor-specific and system-specific keywords = only V%s and Y%s * (except "YA" which is an asset tag) formatted values are accepted. * - * Returns: true if a keyword has been updated successfully, false otherwi= se. + * Unknown or malformed values are ignored. */ -bool -virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, - const char *const keyword, const char *cons= t value) +void +virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, + const bool readOnly, + const char *const keyword, + const char *const value) { if (readOnly) { if (STREQ("EC", keyword) || STREQ("change_level", keyword)) { g_free(res->ro->change_level); res->ro->change_level =3D g_strdup(value); - return true; } else if (STREQ("MN", keyword) || STREQ("manufacture_id", keyword= )) { g_free(res->ro->manufacture_id); res->ro->manufacture_id =3D g_strdup(value); - return true; } else if (STREQ("PN", keyword) || STREQ("part_number", keyword)) { g_free(res->ro->part_number); res->ro->part_number =3D g_strdup(value); - return true; } else if (STREQ("SN", keyword) || STREQ("serial_number", keyword)= ) { g_free(res->ro->serial_number); res->ro->serial_number =3D g_strdup(value); - return true; } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { virPCIVPDResourceCustomUpsertValue(res->ro->vendor_specific, k= eyword[1], value); - return true; } else if (STREQ("FG", keyword) || STREQ("LC", keyword) || STREQ("= PG", keyword)) { /* Legacy PICMIG keywords are skipped on purpose. */ - return true; } else if (STREQ("CP", keyword)) { /* The CP keyword is currently not supported and is skipped. */ - return true; + } else { + VIR_DEBUG("unhandled PCI VPD r/o keyword '%s'(val=3D'%s')", ke= yword, value); } } else { if (STREQ("YA", keyword) || STREQ("asset_tag", keyword)) { g_free(res->rw->asset_tag); res->rw->asset_tag =3D g_strdup(value); - return true; } else if (virPCIVPDResourceIsVendorKeyword(keyword)) { virPCIVPDResourceCustomUpsertValue(res->rw->vendor_specific, k= eyword[1], value); - return true; } else if (virPCIVPDResourceIsSystemKeyword(keyword)) { virPCIVPDResourceCustomUpsertValue(res->rw->system_specific, k= eyword[1], value); - return true; + } else { + VIR_DEBUG("unhandled PCI VPD r/w keyword '%s'(val=3D'%s')", ke= yword, value); } } - VIR_WARN("Tried to update an unsupported keyword %s: skipping.", keywo= rd); - return true; } #ifdef __linux__ @@ -527,10 +521,7 @@ virPCIVPDParseVPDLargeResourceFields(int vpdFileFd, ui= nt16_t resPos, uint16_t re res->rw =3D virPCIVPDResourceRWNew(); } /* The field format, keyword and value are determined. Attempt to = update the resource. */ - if (!virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, f= ieldValue)) { - VIR_INFO("Could not update the VPD resource keyword: %s", fiel= dKeyword); - return false; - } + virPCIVPDResourceUpdateKeyword(res, readOnly, fieldKeyword, fieldV= alue); } /* May have exited the loop prematurely in case RV or RW were encounte= red and diff --git a/src/util/virpcivpd.h b/src/util/virpcivpd.h index 9bfec43e03..d8d3dd3075 100644 --- a/src/util/virpcivpd.h +++ b/src/util/virpcivpd.h @@ -67,9 +67,11 @@ void virPCIVPDResourceRWFree(virPCIVPDResourceRW *rw); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVPDResourceRW, virPCIVPDResourceRWFree= ); -bool -virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, const bool readOnly, - const char *const keyword, const char *cons= t value); +void +virPCIVPDResourceUpdateKeyword(virPCIVPDResource *res, + const bool readOnly, + const char *const keyword, + const char *const value); void virPCIVPDResourceCustomFree(virPCIVPDResourceCustom *custom); diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c index 20545759d5..a6311bfe76 100644 --- a/tests/virpcivpdtest.c +++ b/tests/virpcivpdtest.c @@ -84,17 +84,15 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED) /* Update keywords one by one and compare actual values with the expec= ted ones. */ for (i =3D 0; i < numROCases; ++i) { - if (!virPCIVPDResourceUpdateKeyword(res, true, - readOnlyCases[i].keyword, - readOnlyCases[i].value)) - return -1; + virPCIVPDResourceUpdateKeyword(res, true, + readOnlyCases[i].keyword, + readOnlyCases[i].value); if (STRNEQ(readOnlyCases[i].value, *readOnlyCases[i].actual)) return -1; } /* Do a basic vendor field check. */ - if (!virPCIVPDResourceUpdateKeyword(res, true, "V0", "vendor0")) - return -1; + virPCIVPDResourceUpdateKeyword(res, true, "V0", "vendor0"); if (res->ro->vendor_specific->len !=3D 1) return -1; @@ -105,25 +103,23 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSE= D) /* Make sure unsupported RO keyword updates are not fatal. */ for (i =3D 0; i < numUnsupportedCases; ++i) { - if (!virPCIVPDResourceUpdateKeyword(res, true, - unsupportedFieldCases[i].keywo= rd, - unsupportedFieldCases[i].value= )) - return -1; + virPCIVPDResourceUpdateKeyword(res, true, + unsupportedFieldCases[i].keyword, + unsupportedFieldCases[i].value); } /* Initialize RW */ res->rw =3D g_steal_pointer(&rw); - if (!virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1") - || STRNEQ(res->rw->asset_tag, "tag1")) + virPCIVPDResourceUpdateKeyword(res, false, "YA", "tag1"); + if (STRNEQ(res->rw->asset_tag, "tag1")) return -1; - if (!virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag2") - || STRNEQ(res->rw->asset_tag, "tag2")) + virPCIVPDResourceUpdateKeyword(res, false, "asset_tag", "tag2"); + if (STRNEQ(res->rw->asset_tag, "tag2")) return -1; /* Do a basic system field check. */ - if (!virPCIVPDResourceUpdateKeyword(res, false, "Y0", "system0")) - return -1; + virPCIVPDResourceUpdateKeyword(res, false, "Y0", "system0"); if (res->rw->system_specific->len !=3D 1) return -1; @@ -134,10 +130,12 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSE= D) /* Make sure unsupported RW keyword updates are not fatal. */ for (i =3D 0; i < numUnsupportedCases; ++i) { - if (!virPCIVPDResourceUpdateKeyword(res, false, - unsupportedFieldCases[i].keywo= rd, - unsupportedFieldCases[i].value= )) - return -1; + /* This test is deliberately left in despite + * virPCIVPDResourceUpdateKeyword always succeeding to prevent + * possible regressions if the function is ever rewritten */ + virPCIVPDResourceUpdateKeyword(res, false, + unsupportedFieldCases[i].keyword, + unsupportedFieldCases[i].value); } --=20 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org