From nobody Fri May 3 08:14:30 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 1523341654322428.92621239541927; Mon, 9 Apr 2018 23:27:34 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DEEAB81DEC; Tue, 10 Apr 2018 06:27:32 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7A71C9CCB; Tue, 10 Apr 2018 06:27:32 +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 A382A1800CA2; Tue, 10 Apr 2018 06:27:31 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w3A6RUZa025996 for ; Tue, 10 Apr 2018 02:27:30 -0400 Received: by smtp.corp.redhat.com (Postfix) id 5F1775C688; Tue, 10 Apr 2018 06:27:30 +0000 (UTC) Received: from mx1.redhat.com (ext-mx10.extmail.prod.ext.phx2.redhat.com [10.5.110.39]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 31D6A5C582; Tue, 10 Apr 2018 06:27:27 +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 AE0A95D687; Tue, 10 Apr 2018 06:27:23 +0000 (UTC) Received: from bart.luffy.cx (localhost [127.0.0.1]) by bart.luffy.cx (Postfix) with ESMTP id 6BCAB142F3; Tue, 10 Apr 2018 08:27:21 +0200 (CEST) Received: from neo.luffy.cx (243.250.105.92.dynamic.wline.res.cust.swisscom.ch [92.105.250.243]) by bart.luffy.cx (Postfix) with ESMTPS id 3F52714168; Tue, 10 Apr 2018 08:27:21 +0200 (CEST) Received: by neo.luffy.cx (Postfix, from userid 500) id E767737E6; Tue, 10 Apr 2018 08:27:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=bernat.im; h=from:to:cc :subject:date:message-id:in-reply-to:references; s=postfix; bh=d IsAgcuqHj4StBG1KFxCl27F+WU=; b=BEB/kgvxdc34OlOEHetA0urxILAPm3G12 IWma0okPe94AYbdVinbALHv3BdfjfvFWrAG3RJDoCJ31roxghGvq5G1k1Y6mNnWT PlL8q3x7hIluFbVGZRUj18dLvqFTwHqm1hsvOK7D/fY/5lOPZfaYGWb0agyODgxn 4WdhH3b1+s= DomainKey-Signature: a=rsa-sha1; c=nofws; d=bernat.im; h=from:to:cc :subject:date:message-id:in-reply-to:references; q=dns; s= postfix; b=WGw7Yfz9DGp+B6X5NpL3cS91tKmm8Nuk0/hsLXKMpwU5+30Skcfx0 I9S0TFOgUWkFE2VYGpq/KjIF4AuAL3v2GxWxyR7GnM6rL9d7NT1Rg0bcdW16MKGm H3Wx8gwbZL4zSf176s6kUwLFU4TyVS7O4dXosr+pygixK1gIXVN9sY= From: Vincent Bernat To: libvir-list@redhat.com, Peter Krempa , Michal Privoznik Date: Tue, 10 Apr 2018 08:27:15 +0200 Message-Id: <20180410062715.19050-1-vincent@bernat.im> In-Reply-To: References: X-Greylist: Sender passed SPF test, ACL 227 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 10 Apr 2018 06:27:25 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 10 Apr 2018 06:27:25 +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.149 (DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS, SPF_PASS) 78.47.78.131 bart.luffy.cx 78.47.78.131 bart.luffy.cx X-Scanned-By: MIMEDefang 2.78 on 10.5.110.39 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: Vincent Bernat Subject: [libvirt] [PATCH v2] 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 10 Apr 2018 06:27:33 +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 | 83 --------------------------------------------- 2 files changed, 120 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..e9c03c1afbbc 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) { @@ -303,61 +277,6 @@ testHashSteal(const void *data ATTRIBUTE_UNUSED) } =20 =20 -static int -testHashIter(void *payload ATTRIBUTE_UNUSED, - const void *name ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) -{ - return 0; -} - -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 +547,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