From: Noah Misch <noah@leadboat.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Test pg_atomic_fetch_add_ with variable addend and 16-bit edge c
Date: Sun, 15 Sep 2019 15:14:50 -0700
Message-ID: <20190915221450.GA3500204@rfd.leadboat.com> (raw)
In-Reply-To: <20190915160021.GA24376@alvherre.pgsql>
References: <E1i8y0J-00076R-Jj@gemulon.postgresql.org>
<20190915160021.GA24376@alvherre.pgsql>
On Sun, Sep 15, 2019 at 01:00:21PM -0300, Alvaro Herrera wrote:
> On 2019-Sep-14, Noah Misch wrote:
> > Test pg_atomic_fetch_add_ with variable addend and 16-bit edge cases.
>
> I don't understand this.
>
> if (pg_atomic_fetch_add_u32(&var, INT_MAX) != INT_MAX)
> elog(ERROR, "pg_atomic_add_fetch_u32() #3 wrong");
>
> pg_atomic_fetch_add_u32(&var, 2); /* wrap to 0 */
>
> if (pg_atomic_fetch_add_u32(&var, PG_INT16_MAX) != 0)
> elog(ERROR, "pg_atomic_fetch_add_u32() #3 wrong");
>
> The existing one seems to be naming the wrong function in the error
> message, and if you fix that then you have two "#3 wrong" cases.
> Isn't this confusing? Am I just too sensitive? Is this pointless to
> fix?
It's a typo-class defect. Would you like me to fix it, or would you like to
fix it? I'd consider wrapping the tests in a macro (may be overkill, since
these tests don't change much):
inline patch 1: src/test/regress/regress.c (+13/-10 lines) download inline-1.patch--- a/src/test/regress/regress.c+++ b/src/test/regress/regress.c@@ -670,6 +670,16 @@ test_atomic_flag(void)
pg_atomic_clear_flag(&flag);
}
+#define EXPECT(result_expr, expected_expr) \+ do { \+ uint32 result = (result_expr); \+ uint32 expected = (expected_expr); \+ if (result != expected) \+ elog(ERROR, \+ "%s yielded %u, expected %s in file \"%s\" line %u", \+ #result_expr, result, #expected_expr, __FILE__, __LINE__); \+ } while (0)+
static void
test_atomic_uint32(void)
{
@@ -678,17 +688,10 @@ test_atomic_uint32(void)
int i;
pg_atomic_init_u32(&var, 0);
-- if (pg_atomic_read_u32(&var) != 0)- elog(ERROR, "atomic_read_u32() #1 wrong");-+ EXPECT(pg_atomic_read_u32(&var), 0);
pg_atomic_write_u32(&var, 3);
-- if (pg_atomic_read_u32(&var) != 3)- elog(ERROR, "atomic_read_u32() #2 wrong");-- if (pg_atomic_fetch_add_u32(&var, pg_atomic_read_u32(&var) - 2) != 3)- elog(ERROR, "atomic_fetch_add_u32() #1 wrong");+ EXPECT(pg_atomic_read_u32(&var), 3);+ EXPECT(pg_atomic_fetch_add_u32(&var, pg_atomic_read_u32(&var) - 2), 3);
if (pg_atomic_fetch_sub_u32(&var, 1) != 4)
elog(ERROR, "atomic_fetch_sub_u32() #1 wrong");
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: noah@leadboat.com, alvherre@2ndquadrant.com, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Test pg_atomic_fetch_add_ with variable addend and 16-bit edge c
In-Reply-To: <20190915221450.GA3500204@rfd.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox