| Message ID | 075b3d08-4270-4064-8103-1fece055e197@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] t-strvec: use test_msg() | expand |
René Scharfe <l.s.r@web.de> writes: > diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c > index fdb28ba220..6a4d425840 100644 > --- a/t/unit-tests/t-strvec.c > +++ b/t/unit-tests/t-strvec.c > @@ -8,7 +8,9 @@ > if (check_uint(ARRAY_SIZE(expect), >, 0) && \ > check_pointer_eq(expect[ARRAY_SIZE(expect) - 1], NULL) && \ > check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \ > - check_uint((vec)->nr, <=, (vec)->alloc)) { \ > + ((vec)->v == empty_strvec ? \ > + check_uint((vec)->nr, ==, (vec)->alloc) : \ > + check_uint((vec)->nr, <, (vec)->alloc))) { \ Not a huge deal but with empty_strvec, don't we want to barf if nr==alloc==1? > for (size_t i = 0; i < ARRAY_SIZE(expect); i++) { \ > if (!check_str((vec)->v[i], expect[i])) { \ > test_msg(" i: %"PRIuMAX, i); \ > -- > 2.45.2
Am 15.07.24 um 16:43 schrieb Junio C Hamano: > René Scharfe <l.s.r@web.de> writes: > >> diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c >> index fdb28ba220..6a4d425840 100644 >> --- a/t/unit-tests/t-strvec.c >> +++ b/t/unit-tests/t-strvec.c >> @@ -8,7 +8,9 @@ >> if (check_uint(ARRAY_SIZE(expect), >, 0) && \ >> check_pointer_eq(expect[ARRAY_SIZE(expect) - 1], NULL) && \ >> check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \ >> - check_uint((vec)->nr, <=, (vec)->alloc)) { \ >> + ((vec)->v == empty_strvec ? \ >> + check_uint((vec)->nr, ==, (vec)->alloc) : \ >> + check_uint((vec)->nr, <, (vec)->alloc))) { \ > > Not a huge deal but with empty_strvec, don't we want to barf if > nr==alloc==1? Yes, and that's handled by the comparison with ARRAY_SIZE(expect) - 1 above. Perhaps comparing to 0 explicitly would be clearer. My thinking was just to split up the old <= check into its < and == cases, without changing anything else.. > >> for (size_t i = 0; i < ARRAY_SIZE(expect); i++) { \ >> if (!check_str((vec)->v[i], expect[i])) { \ >> test_msg(" i: %"PRIuMAX, i); \ >> -- >> 2.45.2
René Scharfe <l.s.r@web.de> writes: >>> check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \ >>> - check_uint((vec)->nr, <=, (vec)->alloc)) { \ >>> + ((vec)->v == empty_strvec ? \ >>> + check_uint((vec)->nr, ==, (vec)->alloc) : \ >>> + check_uint((vec)->nr, <, (vec)->alloc))) { \ >> >> Not a huge deal but with empty_strvec, don't we want to barf if >> nr==alloc==1? > > Yes, and that's handled by the comparison with ARRAY_SIZE(expect) - 1 > above. Ah, OK, thanks.
diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c index fdb28ba220..6a4d425840 100644 --- a/t/unit-tests/t-strvec.c +++ b/t/unit-tests/t-strvec.c @@ -8,7 +8,9 @@ if (check_uint(ARRAY_SIZE(expect), >, 0) && \ check_pointer_eq(expect[ARRAY_SIZE(expect) - 1], NULL) && \ check_uint((vec)->nr, ==, ARRAY_SIZE(expect) - 1) && \ - check_uint((vec)->nr, <=, (vec)->alloc)) { \ + ((vec)->v == empty_strvec ? \ + check_uint((vec)->nr, ==, (vec)->alloc) : \ + check_uint((vec)->nr, <, (vec)->alloc))) { \ for (size_t i = 0; i < ARRAY_SIZE(expect); i++) { \ if (!check_str((vec)->v[i], expect[i])) { \ test_msg(" i: %"PRIuMAX, i); \
The members .nr and .alloc of an empty strvec are both zero and the trailing NULL is provided by the shared global variable empty_strvec. Once at least one item is added, .alloc needs to be bigger than .nr to leave room for the terminating NULL. The current .alloc check only tests whether it's bigger or equal to .nr in all cases. Tighten it depending on whether the strvec is backed by empty_strvec or not. A report for a non-zero .alloc in an empty strvec looks like this: # check "(&vec)->nr == (&vec)->alloc" failed at t/unit-tests/t-strvec.c:55 # left: 0 # right: 1 And .alloc being .equal to .nr in a non-empty strvec results in: # check "(&vec)->nr < (&vec)->alloc" failed at t/unit-tests/t-strvec.c:55 # left: 1 # right: 1 Signed-off-by: René Scharfe <l.s.r@web.de> --- Bonus patch. t/unit-tests/t-strvec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.45.2