Revise the Asserts added to bimapset manipulation functions

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Revise the Asserts added to bimapset manipulation functions
Date: 2023-12-27 09:30:01
Message-ID: CAMbWs4-djy9qYux2gZrtmxA0StrYXJjvB-oqLxn-d7J88t=PQQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The Asserts added to bitmapset.c by commits 71a3e8c43b and 7d58f2342b
contain some duplicates, such as in bms_difference, bms_is_subset,
bms_subset_compare, bms_int_members and bms_join. For instance,

@@ -953,6 +1033,15 @@ bms_int_members(Bitmapset *a, const Bitmapset *b)
int lastnonzero;
int shortlen;
int i;
+#ifdef REALLOCATE_BITMAPSETS
+ Bitmapset *tmp = a;
+#endif
+
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));
+
+ Assert(a == NULL || IsA(a, Bitmapset));
+ Assert(b == NULL || IsA(b, Bitmapset));

Sorry that I failed to notice those duplicates when reviewing the
patchset, mainly because they were introduced in different patches.

While removing those duplicates, I think we can add checks in the new
Asserts to ensure that Bitmapsets should not contain trailing zero
words, as the old Asserts did. That makes the Asserts in the form of:

Assert(a == NULL || (IsA(a, Bitmapset) && a->words[a->nwords - 1] != 0));

I think we can define a new macro for this form and use it to check that
a Bitmapset is valid.

In passing, I prefer to move the Asserts to the beginning of functions,
just for paranoia's sake.

Hence, proposed patch attached.

Thanks
Richard

Attachment Content-Type Size
v1-0001-Revise-the-Asserts-added-to-bimapset-manipulation-functions.patch application/octet-stream 10.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2023-12-27 09:38:44 Re: Track in pg_replication_slots the reason why slots conflict?
Previous Message Julien Rouhaud 2023-12-27 08:08:37 Re: pg_stat_statements: more test coverage