From nobody Sun Apr 28 08:06:36 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=fail(p=none dis=none) header.from=bernat.im Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1523001387592359.2437120065757; Fri, 6 Apr 2018 00:56:27 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D0D90804EB; Fri, 6 Apr 2018 07:56:25 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3C300705A8; Fri, 6 Apr 2018 07:56:25 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 642B54CA9C; Fri, 6 Apr 2018 07:56:23 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w367uMcW015854 for ; Fri, 6 Apr 2018 03:56:22 -0400 Received: by smtp.corp.redhat.com (Postfix) id 83494189D3; Fri, 6 Apr 2018 07:56:22 +0000 (UTC) Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7CA8418A53; Fri, 6 Apr 2018 07:56:20 +0000 (UTC) Received: from bart.luffy.cx (bart.luffy.cx [78.47.78.131]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B60A6C057F4E; Fri, 6 Apr 2018 07:56:16 +0000 (UTC) Received: from bart.luffy.cx (localhost [127.0.0.1]) by bart.luffy.cx (Postfix) with ESMTP id 001B31494B; Fri, 6 Apr 2018 09:56:15 +0200 (CEST) Received: from zoro.exoscale.ch (84.212.41.212.static.wline.lns.sme.cust.swisscom.ch [212.41.212.84]) by bart.luffy.cx (Postfix) with ESMTPS id BFCA9146B6; Fri, 6 Apr 2018 09:56:14 +0200 (CEST) Received: by zoro.exoscale.ch (Postfix, from userid 1000) id 6E1813B4; Fri, 6 Apr 2018 09:56:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=bernat.im; h=from:to:cc :subject:date:message-id; s=postfix; bh=+97VL9EQy74/Rfqkvvqx3Sn2 hg0=; b=hCXJ4N1DcHmW19KSgnLmWeIbdp6Gi1QrhyMNNvcj/SkFRSMf8QMoS0FF m/CpxC28uCh/6ZkQIFqRvuOuEXU+YsHiql0+iQwm8VzJGl8KJwLef4yK3foN4aFs fU4DYBblrhpZi1/HvylWgusjC0egGzLvfzCsjUCyZe0VlE58nEE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=bernat.im; h=from:to:cc :subject:date:message-id; q=dns; s=postfix; b=xJexlwkt7Vaq0+HRS6 ycmBh4TT5RMGJxBhmehTUMWSeP4bnKSVsw9AvS+vtJBoVRU6vY69W4AkdGV/G5km 4YKyQhRb8dYkXkA3odB9XhjMDYeVvgsJBo7VwaYPPqV4eb4N59sEjjqqZ0MTaRK4 GMSIRf35+/WtVZJOmJoXLSDA0= From: Vincent Bernat To: libvir-list@redhat.com, Michal Privoznik Date: Fri, 6 Apr 2018 09:56:07 +0200 Message-Id: <20180406075607.3015-1-vincent@bernat.im> X-Greylist: Sender passed SPF test, ACL 227 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 06 Apr 2018 07:56:18 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 06 Apr 2018 07:56:18 +0000 (UTC) for IP:'78.47.78.131' DOMAIN:'bart.luffy.cx' HELO:'bart.luffy.cx' FROM:'bernat@luffy.cx' RCPT:'' X-RedHat-Spam-Score: 0.139 (DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, SPF_PASS, T_RP_MATCHES_RCVD) 78.47.78.131 bart.luffy.cx 78.47.78.131 bart.luffy.cx X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Cc: Vincent Bernat Subject: [libvirt] [PATCH] util: don't check for parallel iteration in hash-related functions 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 06 Apr 2018 07:56:26 +0000 (UTC) X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This is the responsability of the caller to apply the correct lock before using these functions. Moreover, the use of a simple boolean was still racy: two threads may check the boolean and "lock" it simultaneously. Users of functions from src/util/virhash.c have to be checked for correctness. Lookups and iteration should hold a RO lock. Modifications should hold a RW lock. Most important uses seem to be covered. Callers have now a greater responsability, notably the ability to execute some operations while iterating were reliably forbidden before are now accepted. --- src/util/virhash.c | 37 ---------------------- tests/virhashtest.c | 75 --------------------------------------------- 2 files changed, 112 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index 0ffbfcce2c64..475c2b0281b2 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -41,12 +41,6 @@ VIR_LOG_INIT("util.hash"); =20 /* #define DEBUG_GROW */ =20 -#define virHashIterationError(ret) \ - do { \ - VIR_ERROR(_("Hash operation not allowed during iteration")); \ - return ret; \ - } while (0) - /* * A single entry in the hash table */ @@ -66,10 +60,6 @@ struct _virHashTable { uint32_t seed; size_t size; size_t nbElems; - /* True iff we are iterating over hash entries. */ - bool iterating; - /* Pointer to the current entry during iteration. */ - virHashEntryPtr current; virHashDataFree dataFree; virHashKeyCode keyCode; virHashKeyEqual keyEqual; @@ -339,9 +329,6 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const vo= id *name, if ((table =3D=3D NULL) || (name =3D=3D NULL)) return -1; =20 - if (table->iterating) - virHashIterationError(-1); - key =3D virHashComputeKey(table, name); =20 /* Check for duplicate entry */ @@ -551,9 +538,6 @@ virHashRemoveEntry(virHashTablePtr table, const void *n= ame) nextptr =3D table->table + virHashComputeKey(table, name); for (entry =3D *nextptr; entry; entry =3D entry->next) { if (table->keyEqual(entry->name, name)) { - if (table->iterating && table->current !=3D entry) - virHashIterationError(-1); - if (table->dataFree) table->dataFree(entry->payload, entry->name); if (table->keyFree) @@ -593,18 +577,11 @@ virHashForEach(virHashTablePtr table, virHashIterator= iter, void *data) if (table =3D=3D NULL || iter =3D=3D NULL) return -1; =20 - if (table->iterating) - virHashIterationError(-1); - - table->iterating =3D true; - table->current =3D NULL; for (i =3D 0; i < table->size; i++) { virHashEntryPtr entry =3D table->table[i]; while (entry) { virHashEntryPtr next =3D entry->next; - table->current =3D entry; ret =3D iter(entry->payload, entry->name, data); - table->current =3D NULL; =20 if (ret < 0) goto cleanup; @@ -615,7 +592,6 @@ virHashForEach(virHashTablePtr table, virHashIterator i= ter, void *data) =20 ret =3D 0; cleanup: - table->iterating =3D false; return ret; } =20 @@ -643,11 +619,6 @@ virHashRemoveSet(virHashTablePtr table, if (table =3D=3D NULL || iter =3D=3D NULL) return -1; =20 - if (table->iterating) - virHashIterationError(-1); - - table->iterating =3D true; - table->current =3D NULL; for (i =3D 0; i < table->size; i++) { virHashEntryPtr *nextptr =3D table->table + i; =20 @@ -667,7 +638,6 @@ virHashRemoveSet(virHashTablePtr table, } } } - table->iterating =3D false; =20 return count; } @@ -723,23 +693,16 @@ void *virHashSearch(const virHashTable *ctable, if (table =3D=3D NULL || iter =3D=3D NULL) return NULL; =20 - if (table->iterating) - virHashIterationError(NULL); - - table->iterating =3D true; - table->current =3D NULL; for (i =3D 0; i < table->size; i++) { virHashEntryPtr entry; for (entry =3D table->table[i]; entry; entry =3D entry->next) { if (iter(entry->payload, entry->name, data)) { - table->iterating =3D false; if (name) *name =3D table->keyCopy(entry->name); return entry->payload; } } } - table->iterating =3D false; =20 return NULL; } diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 3b85b62c301d..a013bc716943 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -221,32 +221,6 @@ testHashRemoveForEachAll(void *payload ATTRIBUTE_UNUSE= D, } =20 =20 -const int testHashCountRemoveForEachForbidden =3D ARRAY_CARDINALITY(uuids); - -static int -testHashRemoveForEachForbidden(void *payload ATTRIBUTE_UNUSED, - const void *name, - void *data) -{ - virHashTablePtr hash =3D data; - size_t i; - - for (i =3D 0; i < ARRAY_CARDINALITY(uuids_subset); i++) { - if (STREQ(uuids_subset[i], name)) { - int next =3D (i + 1) % ARRAY_CARDINALITY(uuids_subset); - - if (virHashRemoveEntry(hash, uuids_subset[next]) =3D=3D 0) { - VIR_TEST_VERBOSE( - "\nentry \"%s\" should not be allowed to be remove= d", - uuids_subset[next]); - } - break; - } - } - return 0; -} - - static int testHashRemoveForEach(const void *data) { @@ -311,53 +285,6 @@ testHashIter(void *payload ATTRIBUTE_UNUSED, return 0; } =20 -static int -testHashForEachIter(void *payload ATTRIBUTE_UNUSED, - const void *name ATTRIBUTE_UNUSED, - void *data) -{ - virHashTablePtr hash =3D data; - - if (virHashAddEntry(hash, uuids_new[0], NULL) =3D=3D 0) - VIR_TEST_VERBOSE("\nadding entries in ForEach should be forbidden"= ); - - if (virHashUpdateEntry(hash, uuids_new[0], NULL) =3D=3D 0) - VIR_TEST_VERBOSE("\nupdating entries in ForEach should be forbidde= n"); - - if (virHashSteal(hash, uuids_new[0]) !=3D NULL) - VIR_TEST_VERBOSE("\nstealing entries in ForEach should be forbidde= n"); - - if (virHashSteal(hash, uuids_new[0]) !=3D NULL) - VIR_TEST_VERBOSE("\nstealing entries in ForEach should be forbidde= n"); - - if (virHashForEach(hash, testHashIter, NULL) >=3D 0) - VIR_TEST_VERBOSE("\niterating through hash in ForEach" - " should be forbidden"); - return 0; -} - -static int -testHashForEach(const void *data ATTRIBUTE_UNUSED) -{ - virHashTablePtr hash; - int ret =3D -1; - - if (!(hash =3D testHashInit(0))) - return -1; - - if (virHashForEach(hash, testHashForEachIter, hash)) { - VIR_TEST_VERBOSE("\nvirHashForEach didn't go through all entries"); - goto cleanup; - } - - ret =3D 0; - - cleanup: - virHashFree(hash); - return ret; -} - - static int testHashRemoveSetIter(const void *payload ATTRIBUTE_UNUSED, const void *name, @@ -628,9 +555,7 @@ mymain(void) DO_TEST("Remove", Remove); DO_TEST_DATA("Remove in ForEach", RemoveForEach, Some); DO_TEST_DATA("Remove in ForEach", RemoveForEach, All); - DO_TEST_DATA("Remove in ForEach", RemoveForEach, Forbidden); DO_TEST("Steal", Steal); - DO_TEST("Forbidden ops in ForEach", ForEach); DO_TEST("RemoveSet", RemoveSet); DO_TEST("Search", Search); DO_TEST("GetItems", GetItems); --=20 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list